-
Notifications
You must be signed in to change notification settings - Fork 198
[AURON #1792] Keep the null result in the reverse connection result #1793
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR fixes a bug in Left Anti Join handling where NULL join keys were incorrectly included in the result set. The fix ensures NULL join keys are filtered out from Anti join results, which aligns with SQL NOT IN semantics where NULL NOT IN (...) evaluates to NULL (treated as false).
Key Changes
- Added explicit NULL join key handling for Anti joins that filters out rows with NULL keys
- Refactored the key validity check to be computed once before the conditional logic
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if P.mode == Anti && P.probe_is_join_side && !key_is_valid { | ||
| probed_joined.set(row_idx, true); | ||
| continue; | ||
| } |
Copilot
AI
Dec 26, 2025
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.
This fix correctly handles NULL join keys in Anti joins by filtering them out, which matches SQL NOT IN semantics. However, there's no test coverage for this specific scenario. Consider adding a test case for Anti joins with NULL join keys to prevent regression. The existing test in test.rs (join_anti function) only tests non-null keys.
Which issue does this PR close?
Closes #1792
Rationale for this change
Keep the null result in the reverse connection result
What changes are included in this PR?
Are there any user-facing changes?
How was this patch tested?
cluster test