Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert Null Join Filter #745

Merged
merged 2 commits into from
Sep 11, 2020
Merged

Conversation

revans2
Copy link
Collaborator

@revans2 revans2 commented Sep 11, 2020

This fixes #744

This reverts #594

It appears that I missed something when I put in the original change and I cannot think of what I did wrong quickly, so I am going to revert this for 0.2 and reopen the original issue so I can try again in 0.3

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2 revans2 added the bug Something isn't working label Sep 11, 2020
@revans2 revans2 added this to the Aug 31 - Sep 11 milestone Sep 11, 2020
@revans2 revans2 self-assigned this Sep 11, 2020
@revans2
Copy link
Collaborator Author

revans2 commented Sep 11, 2020

build

@jlowe jlowe changed the title Revert Null Join Fitler Revert Null Join Filter Sep 11, 2020
@revans2
Copy link
Collaborator Author

revans2 commented Sep 11, 2020

I reran all of the tpc-ds tests and scale factor 1 and they all pass now.

Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunate that some nice code cleanup is also getting reverted with this.

@revans2
Copy link
Collaborator Author

revans2 commented Sep 11, 2020

Unfortunate that some nice code cleanup is also getting reverted with this.

I plan on putting this back in again for 0.3 once I have more then an hour to try and figure out what is happening.

@jlowe
Copy link
Member

jlowe commented Sep 11, 2020

Actually now that this is getting punted to 0.3 and thus cudf-0.16, I wonder if we should focus our efforts on implementing the fix in libcudf as mentioned by rapidsai/cudf#6052 (comment). I think that would be the preferred solution from a performance and code complexity perspective.

@revans2 revans2 merged commit ebe10a6 into NVIDIA:branch-0.2 Sep 11, 2020
@revans2 revans2 deleted the revert-nulljoin-filter branch September 11, 2020 22:06
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* Revert "Filter nulls from joins where possible to improve performance. (NVIDIA#594)"

This reverts commit df00904.

* Fix tests to run

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* Revert "Filter nulls from joins where possible to improve performance. (NVIDIA#594)"

This reverts commit df00904.

* Fix tests to run

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
[auto-merge] bot-auto-merge-branch-22.12 to branch-23.02 [skip ci] [bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] TPC-DS query 6 now produces incorrect results.
3 participants