Repository navigation
[branch-56] fix: reject a multi-key null-aware join without a NOT IN value key count (backport #27111) - #27176
Merged
Conversation
…unt (apache#27111) - Closes apache#26569. leading keys that are `NOT IN` value keys (the rest are correlation scope keys), and defaulted it to `1` in `HashJoinExecBuilder::new`, `Join::try_new` and the `LogicalPlanBuilder` join methods. A caller that builds `(a, b) NOT IN (SELECT x, y ...)` itself and only sets `with_null_aware(true)` (for example an external adaptive planner) silently gets one value key, with `b` read as a scope key. The outer row `(1, NULL)` against the subquery row `(1, 5)` is then kept, although its `NOT IN` is UNKNOWN and it must be dropped. 55.x rejected such a plan ("null_aware anti join only supports single column join key"). DataFusion's own planning always sets the count, so SQL is not affected. - `null_aware_value_keys` stays a `usize`, and `0` now means unset. It is the new default of `Join::try_new`, the `LogicalPlanBuilder` join methods, `HashJoinExecBuilder::new` and `HashJoinExec::try_new`. - An unset count is accepted only for a single-key null-aware join, which is a scalar `NOT IN`. A null-aware join with more keys and no count fails to build, with an error asking for `with_null_aware_value_keys`. There is deliberately no "all keys are value keys" default: that is just as wrong for a correlated subquery. - The join type, partition mode and an empty `on` are checked before the count, so a join that no count can fix reports its own error. - Decorrelation sets the count only on a null-aware join, so every other join keeps it unset, as `Join::try_new` leaves it. - The logical and physical proto decoders drop their `.max(1)`, so an unset count round-trips as unset. A payload predating the field (55.x, which only allowed single-key null-aware joins) decodes it as `0` and still reads as a scalar `NOT IN`. The proto field types are unchanged; only their comments are updated. - The 56.0.0 upgrade guide section is shortened. - `test_null_aware_validation_value_keys` (`hash_join/exec.rs`): a two-key null-aware join with an unset or out-of-range count fails to build, `HashJoinExec::try_new` rejects a two-key null-aware join, a single-key join with an unset count builds as a scalar `NOT IN`, and a join that no count can fix (a two-key `Inner`, a `Partitioned` `RightAnti` or `LeftAnti`, an empty `on`) reports its own error. - `null_aware_value_keys_round_trip` (physical proto): an unset count is rejected on decode with two keys and kept unset with one. - `decorrelated_mark_join_sets_value_keys_only_when_null_aware` (core `sql/joins.rs`): scalar and multi-column `IN` mark joins on non-nullable keys keep the count unset, and their null-aware counterparts carry `1` and `2`. - Existing physical tests that built correlated scalar `NOT IN` joins through `HashJoinExec::try_new` and relied on the old default now state the count through a test helper. - Reverting each fix makes the matching test fail: the unset-multi-key check, the check order, the decoders' `.max(1)` and the decorrelation guard. Yes, for code that builds null-aware joins directly: a multi-key null-aware join must set `with_null_aware_value_keys`; it no longer silently defaults to one value key. A struct literal that sets the field to `1` for a non-null-aware join still works, but `0` is now the default. The field is new in 56, and the upgrade guide is updated. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com> (cherry picked from commit 22aedf0)
github-actions
Bot
added
documentation
28 of 53 tasks
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## branch-56 #27176 +/- ##
==========================================
Coverage 83.26% 83.26%
==========================================
Files 1169 1169
Lines 474209 474282 +73
Branches 474209 474282 +73
==========================================
+ Hits 394853 394918 +65
- Misses 55458 55465 +7
- Partials 23898 23899 +1
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
mbutrovich
approved these changes
Nov 6, 2026
mbutrovich
left a comment
mbutrovich
left a comment
Contributor
There was a problem hiding this comment.
Looks like a clean cherry-pick, approved pending CI. Thanks @alamb!
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
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?
branch-5656.0.0(Oct/Nov 2026) #24461