fix(physical-plan): report Final emission for LeftSemi hash and neste… - #24976
Open
TinyMurky wants to merge 2 commits into
Open
fix(physical-plan): report Final emission for LeftSemi hash and neste…#24976TinyMurky wants to merge 2 commits into
TinyMurky wants to merge 2 commits into
Conversation
…d loop joins ## Which issue does this PR close? - apache#24962(apache#24962) ## Rationale for this change `HashJoinExec` and `NestedLoopJoinExec` report `LeftSemi` joins as emitting incrementally, but they only emit matched build-side rows after the probeside is exhausted. Reporting `EmissionType::Final` reflects their actual behavior and allows `SanityCheckPlan` to reject pipelines with an unbounded probe side that cannot produce output. ## What changes are included in this PR? - Classify `LeftSemi` joins as `EmissionType::Final` in both operators, regardless of the probe input's emission type. - Added unit tests verifying the emission type of LeftSemi joins in both `HashJoinExec` and `NestedLoopJoinExec`. - Add an integration test covering input swapping for an unbounded left input, rejection of an unbounded probe input, and successful planning when both inputs are bounded. ## Are these changes tested? - Added unit tests verifying the emission type of LeftSemi joins in both `HashJoinExec` and `NestedLoopJoinExec`. - Added an integration test covering: - Unbounded left and bounded right: planning succeeds after swapping to RightSemi. - Bounded left and unbounded right: planning is rejected. - Both inputs bounded: planning succeeds. Following test commands have been executed and passed - `cargo test -p datafusion` - `cargo test --profile=ci --test sqllogictests` - `cargo test -p datafusion` - `cargo test -p datafusion-cli` ## Are there any user-facing changes? Yes. `LeftSemi` hash and nested loop joins now report Final emission.Plans with a bounded build side and an unbounded probe side are rejected instead of being accepted despite being unable to produce output.Results for bounded inputs are unchanged.
github-actions
Bot
added
core
jayzhan211
approved these changes
Sep 6, 2026
jayzhan211
left a comment
jayzhan211
left a comment
Contributor
There was a problem hiding this comment.
Thanks @TinyMurky , LGTM!
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24976 +/- ##
==========================================
- Coverage 81.67% 81.67% -0.01%
==========================================
Files 1126 1126
Lines 414533 414560 +27
Branches 414533 414560 +27
==========================================
+ Hits 338562 338581 +19
+ Misses 56058 56057 -1
- Partials 19913 19922 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Author
|
Thanks for the review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
LeftSemihash and nested loop joins reportEmissionType::Incrementalbut emit only after the probe side is exhausted #24962Rationale for this change
HashJoinExecandNestedLoopJoinExecreportLeftSemijoins as emitting incrementally, but they only emit matched build-side rows after the probeside is exhausted.
Reporting
EmissionType::Finalreflects their actual behavior and allowsSanityCheckPlanto reject pipelines with an unbounded probe side that cannot produce output.What changes are included in this PR?
LeftSemijoins asEmissionType::Finalin both operators, regardless of the probe input's emission type.HashJoinExecandNestedLoopJoinExec.input, rejection of an unbounded probe input, and successful planning when
both inputs are bounded.
Are these changes tested?
Added unit tests verifying the emission type of LeftSemi joins in both
HashJoinExecandNestedLoopJoinExec.Added an integration test covering:
RightSemi.
Following test commands have been executed and passed
cargo test -p datafusioncargo test --profile=ci --test sqllogictestscargo test -p datafusioncargo test -p datafusion-cliAre there any user-facing changes?
Yes.
LeftSemihash and nested loop joins now report Final emission.Plans with a bounded build side and an unbounded probe side are rejected instead of being accepted despite being unable to produce output.
Results for bounded inputs are unchanged.