-
Notifications
You must be signed in to change notification settings - Fork 928
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
Add distinct left join #15149
Add distinct left join #15149
Conversation
Benchmark results on RTX8000: left_join_32bit[0] Quadro RTX 8000
distinct_left_join_32bit[0] Quadro RTX 8000
|
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.
Do we need tests that explicitly compare distinct join results to "normal" join results? I might have overlooked that, but it would be good to confirm that the results match. (Distinct joins should just be faster since they have an additional requirement on the input.)
Distinct join results are always compared against "gold references" instead of the output of normal joins in unit tests. It's a more reliable verification since "normal" joins can go wrong as well. |
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.
I just have a few questions/nits. Overall LGTM.
Co-authored-by: Nghia Truong <7416935+ttnghia@users.noreply.github.com>
/merge |
Adds Java bindings to the distinct left join functionality added in #15149. Authors: - Jason Lowe (https://github.com/jlowe) Approvers: - Robert (Bobby) Evans (https://github.com/revans2) - Jim Brennan (https://github.com/jbrennan333) URL: #15154
Description
Contributes to #14948
This PR adds distinct left join. It also cleans up the distinct inner code to use the terms "build" and "probe" consistently instead of "left" and "right".
Checklist