Repository navigation
fix: drop build rows with a dictionary NULL key in null-aware anti joins - #26578
Merged
Merged
Conversation
jayzhan211
self-requested a review
October 23, 2026 06:57
jayzhan211
added a commit
to cning112/datafusion
that referenced
this pull request
Oct 24, 2026
## Which issue does this PR close? - None. ## Rationale for this change `HashJoinExec` supports SQL `NOT IN` through `null_aware = true`, in three forms: `LeftAnti`, `LeftMark` and `RightAnti`, each correlated or not. The rules for which rows such a join emits are spread out today: - `exec.rs`: `NullAwareMode`, four `JoinLeftData` fields and one `JoinBuildData` field that only some forms use, and the correlated per-row state built inside the build function. - `stream.rs`: about 700 lines inside the regular join stream. These are the batch-skip decision, the correlated marking pass and its helpers, the RightAnti NULL and UNKNOWN drops, and two final-stage functions. - `utils.rs`: a second copy of the final rule, used for the LeftMark column. The two copies of the final rule already drifted once: LeftMark checked logical NULLs since apache#25693, LeftAnti physical ones until apache#26578. This code changes often. Since July it gained four features (apache#23957, apache#21585, apache#19857, apache#26213) and seven correctness fixes (apache#23104, apache#23173, apache#25076, apache#25339, apache#25693, apache#26586, apache#26578), and apache#26214, apache#25679 and apache#26088 propose more. Each such change has to find the `NOT IN` rules in three files, usually about six functions, interleaved with the regular join. A change to the regular probe loop has to read past them too. This PR moves the rules into `hash_join/not_in.rs`, with one copy of the final rule, so a `NOT IN` change touches one module and a regular-join change doesn't need to read it. The join reaches it at four points: 1. `NotIn::try_new` decides when the plan is built or executed whether `null_aware` is legal for the join, and which form it is: `BuildRows { scope, output: Rows | Mark }` for LeftAnti and LeftMark, or `ProbeRows { scope }` for RightAnti, where `scope` is `Uncorrelated` or `Correlated`. 2. `NotInBuild::try_new` runs once per build. It has one variant per form, holding only that form's state. `JoinLeftData` keeps a single `Option<NotInBuild>`. 3. `NotInProbe::before_lookup` / `after_lookup` run around each probe batch's lookup, and `after_empty_build` replaces `after_lookup` when the build map is empty. They skip the batch, record what the probe saw, mark build rows or this batch's probe rows UNKNOWN, and drop the probe rows a RightAnti must not emit; for other joins they do nothing. 4. `NotInBuild::final_chunk` runs on the build rows emitted at the end. It applies one rule for an unmatched row's mark, used both by LeftAnti (keep the rows whose mark is FALSE) and by LeftMark (the mark column). ## What changes are included in this PR? Six commits, each compiling and passing clippy on its own: 1. **Move the correlated marking helpers into `not_in.rs`.** The code is unchanged, apart from one signature that now takes the build data and probe batch instead of the stream's private state structs. This commit also adds a test for correlated marking when every build value key is NULL. The full-key map is empty then, and the marking must still run before the empty-map shortcut; nothing covered this before. 2. **`NotIn` replaces `NullAwareMode`.** The legality checks, messages and order are the same. 3. **`NotInBuild` holds the per-form build state, with one final rule.** It replaces the `JoinLeftData` / `JoinBuildData` fields, the two final-stage functions in `stream.rs`, and `build_null_aware_left_mark_column` in `utils.rs`. 4. **The `NotInProbe` hooks replace the stream's `NOT IN` branches and fields.** The correlated RightAnti's per-batch UNKNOWN probe rows move from `ProcessProbeBatchState` into `NotInProbe`, which recomputes them on each batch's first lookup. 5. **Review follow-ups.** `HashJoinExec::not_in()` and the `not_in` parameters replace `null_aware_mode()` and `null_aware: Option<NotIn>`, the helpers used only inside `not_in.rs` become private, and `final_chunk` asserts that a probe side with a NULL key is not empty. 6. **Test the correlated RightAnti empty-build path.** `test_null_aware_correlated_all_null_build_values` also runs RightAnti, which must drop the UNKNOWN probe rows when the build map is empty (`NotInProbe::after_empty_build`). Types: `NullAwareMode` goes, and `NotIn`, `NotInScope`, `NotInOutput`, `NotInBuild` and `NotInProbe` come in. Gone: the `NOT IN` fields of `JoinLeftData`, `JoinBuildData`, `HashJoinStream` and `ProcessProbeBatchState` and their constructor parameters, 4 stream functions (`null_aware_skip_probe_batch`, `drop_null_probe_keys`, `null_aware_left_anti_final_indices`, `null_aware_left_mark_column`), and the second copy of the final rule. What does not change: - the planner rules, including the `JoinSelection` swap to RightAnti; - the dynamic-filter gate for null-aware joins; - the prefilter and prepared-build exclusions; - the cross-partition `ProbeCompletion`. ### Behavior notes No query results change. Two internal orders do: - **Reservation order.** For an uncorrelated LeftAnti or LeftMark join whose build key has newly materialized logical NULLs (a dictionary, REE or Null key), the NULL buffer is now charged to the memory reservation right after the visited bitmap. On main it was charged after the membership step's optional pruning bitmap. Under a tight memory limit, a different allocation may be the one that fails. The pruning bitmap only exists with a dynamic filter, which a null-aware join with a nullable build key doesn't get. - **The form is computed twice per execution.** The stream reads it from the build (`JoinLeftData::not_in`); `execute` derives it again (`HashJoinExec::not_in()`) for the dynamic filter. Main derived it three times (build, stream, accumulator). The two can only disagree if an executed plan is rebuilt and run again without `reset_state`, which is already unsupported. Main panicked in that case; this PR uses the build's form. ## Performance Benchmark bot (GKE `c4a-highmem-16`, aarch64) on `a9d4896d7e` against its merge base `702803d022`; the later commit only adds a test: - `null_aware_join`: geomean 0.989. Q01 and Q03 are 4% faster, Q02, Q05 and Q06 about 2% faster, Q08 2% slower (20.4 → 20.8 ms), the rest are within 1%. - `hj`: total 6870 → 6885 ms (+0.2%). Q22 and Q24 are unchanged. The queries the bot flags (Q11, Q14, Q15, Q17 slower; Q16, Q18 faster) move within their run-to-run spread. - `tpch10`: all 22 queries unchanged, total 7078 → 7049 ms. Earlier runs on an M-series Mac showed this refactor slowing `hj` Q24 by up to 9% through different LLVM inlining of the probe loop. That doesn't reproduce on the bot, so the PR leaves inlining to the compiler. Not measured on x86_64. ## What is the testing strategy for this PR? - New test: `test_null_aware_correlated_all_null_build_values`, covering LeftAnti, LeftMark and RightAnti with the probe in two batches, at every batch-size config. - Existing tests: - physical-plan unit tests; - the null-aware join fuzz test (`null_aware_join_fuzz`, with `extended_tests`), which covers uncorrelated and correlated RightAnti plans after the `JoinSelection` swap; - the `null_aware_*` and `subquery*` sqllogictest files, with no changes to expected results or `EXPLAIN` output. ## Are there any user-facing changes? No.
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?
Rationale for this change
x NOT IN (subquery)keeps a build row whose key is a dictionary entry pointing at a NULL value.NULL NOT IN (5, 6)is UNKNOWN, so the row should be dropped, as it is when the key is a physical NULL.What changes are included in this PR?
The final step of the uncorrelated null-aware
LeftAntijoin now checks the logical nulls of the build key instead ofis_null. They are computed once with the build side, reusing what #25693 added forLeftMark, so the field and its getter are renamed tonull_aware_build_key_nulls.What is the testing strategy for this PR?
New
test_null_aware_left_anti_build_logical_nullwith the case from the issue. It fails without the fix.Are there any user-facing changes?
No.