-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: duplicate output for HashJoinExec in CollectLeft mode #9757
Conversation
/benchmark |
Benchmark resultsBenchmarks comparing 01ff537 (main) and 5f368c6 (PR)
|
FYI @gruuya -- it is neat to see the benchmark results appear on PRs ❤️ |
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.
Thank you @korowa . This looks like a very nice PR
@Dandandan or @metesynnada do you have time to review this PR?
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.
LGTM @korowa 👍 with this pr ballista will avoid shuffle data to disk in some case.
5f368c6
to
e88e38c
Compare
Thanks @korowa and @Ted-Jiang -- I also gave this PR a brief skim and it looks really nice to me. |
* fix: duplicate output for HashJoinExec in CollectLeft mode * address review comments * test fix after merging main
Which issue does this PR close?
Closes #.
Rationale for this change
While #9676, found that at this moment
HashJoinExec
works incorrect in case of LEFT / FULL types. Currently, construction of these joins is prevented by physical optimizer join_selection rule. This PR adds support for all join types in CollectLeft mode.What changes are included in this PR?
JoinLeftData
now contains bitmap with visited left indices and atomic counter of total right-side threads (streams), updating this particularJoinLeftData
object (initial value will always be 1 for partitioned joins, and "number of right-side partitions" for CollectLeft)process_unmatched_build_batch
, when it's guaranteed that there won't be any further updates of visited indices bitmap, eachHashJoinStream
decrements the counter of running partitions by callingreport_probe_completed
, and only the last caller-thread (determined by counter value) will return unmatched left-side datatry_collect_left
in join_selection ruleAre these changes tested?
Yes, by adding test case for parallel (multiple right-side partitions) hash join execution with CollectLeft partition mode
Are there any user-facing changes?
No