viirya opened a new pull request, #27111:
URL: https://github.com/apache/datafusion/pull/27111
## Which issue does this PR close?
- Closes #26569.
## Rationale for this change
#19857 added `null_aware_value_keys`, the number of a null-aware join's
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.
## What changes are included in this PR?
- `Join::null_aware_value_keys` and `HashJoinExec::null_aware_value_keys`
become `Option<NonZeroUsize>`, unset by default; both
`with_null_aware_value_keys` setters take a `NonZeroUsize`.
- 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.
- A count of zero is now unrepresentable instead of a runtime error.
`NonZeroUsize` also keeps `Option` free of size cost, so `LogicalPlan` stays at
176 bytes (`test_size_of_logical_plan`).
- The physical planner, `eliminate_join` and the other rules pass the count
through unchanged.
- The `JoinNode` and `HashJoinExecNode` proto fields become `optional
uint32`, the same as `fetch`. An unset count round-trips as unset, a payload
predating the field (55.x, which only allowed single-key null-aware joins)
still decodes as a scalar `NOT IN`, and a wire value of `0` is rejected. This
replaces the `.max(1)` fallback.
- EXPLAIN output is unchanged: an unset count or a count of 1 still prints
`null_aware`.
- The 56.0.0 upgrade guide section is updated. It also corrects the claim
that a 55 reader would misread a multi-column `NOT IN` plan: 55 rejects it when
building the `HashJoinExec`.
Both fields are new since 55, so changing their type does not break a
released API.
## What is the testing strategy for this PR?
- `test_null_aware_unset_value_keys` (`hash_join/exec.rs`): for `LeftAnti`,
`LeftMark` and `RightAnti`, a two-key null-aware join without a count fails to
build through both `HashJoinExecBuilder` and `HashJoinExec::try_new`, and a
single-key join builds and resolves to one value key.
- `null_aware_value_keys_round_trip` (physical proto): `Some(2)` and unset
both round-trip; an unset count with two keys and a wire count of `0` are both
rejected on decode.
- `roundtrip_null_aware_join_unset_value_keys` (logical proto): a
`Join::try_new` null-aware join keeps an unset count through the round trip,
physical planning accepts it with one key and rejects it with two, and a wire
count of `0` is rejected.
- 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.
- With the unset-multi-key check reverted to the old default, all three new
tests fail.
## Are there any user-facing changes?
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.
- `null_aware_value_keys` is `Option<NonZeroUsize>` on `Join` and
`HashJoinExec`, and `Option<u32>` on the generated proto types.
Both fields are unreleased (new in 56), and the upgrade guide is updated.
This pull request and its description were written by Isaac.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
