-
Notifications
You must be signed in to change notification settings - Fork 902
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
Refactor conditional joins #8815
Refactor conditional joins #8815
Conversation
… outstanding PR comments from PR rapidsai#8214.
@harrism @hyperbolic2346 I've re-requested your review here since I think this is the part of #8783 that requires the least knowledge of the AST internals. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly nits here. Looks good overall.
Codecov Report
@@ Coverage Diff @@
## branch-21.10 #8815 +/- ##
================================================
- Coverage 10.67% 10.59% -0.09%
================================================
Files 110 116 +6
Lines 18271 19037 +766
================================================
+ Hits 1951 2017 +66
- Misses 16320 17020 +700
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, a few nits.
…garding shuffle and warp syncs.
@gpucibot merge |
#8214 made use of a number of functions defined for hash joins, but to expedite the review process no attempt was made to improve the organization of code into shared files. This PR improves that organization for improved conceptual clarity and faster parallel compilation and recompilation times. As a result, most of the changes in this PR are simply preexisting code to new files, as well as removing a number of unnecessary headers. The main new code is a few minor improvements that were suggested but not implemented on #8214 are included, mainly 1) a new
nullable(table_view const&)
API for checking if any columns in a table are nullable (requested here), and 2) some extra comments on the behavior of conditional joins when the condition returns null (requested here).Contributes to #8145.