Skip to content

fix: apply the join filter when a null-aware hash join marks rows UNKNOWN - #25559

Draft
adriangb wants to merge 2 commits into
apache:mainfrom
pydantic:na-executor
Draft

adriangb wants to merge 2 commits into
apache:mainfrom
pydantic:na-executor

Conversation

@adriangb

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Important

Stacked on #25558. GitHub cannot base a PR on another fork's branch, so the diff here shows both. Review only the last commit (fix: apply the join filter when a null-aware hash join marks rows UNKNOWN). Draft until #25558 lands; I will rebase then.

Rationale for this change

A correlated NOT IN whose correlation cannot become an equi-join key leaves a residual join filter. The null-aware hash join ignored that filter when deciding whether a NULL on the subquery side makes NOT IN UNKNOWN, so a NULL the filter excludes still poisoned every outer row:

CREATE TABLE oc(id INT, g INT) AS VALUES (1,5),(2,5),(3,0),(4,NULL),(NULL,5),(NULL,0);
CREATE TABLE ic(id INT) AS VALUES (1),(NULL);
SELECT id, g FROM oc WHERE oc.id NOT IN (SELECT ic.id FROM ic WHERE oc.g > 0);

returns no rows; DuckDB and PostgreSQL return 3|0, 4|NULL, NULL|0. oc.g > 0 holds only for id 1, 2 and the NULL-id row, so only those three see the subquery {1, NULL}; the rest see an empty subquery, and NOT IN over an empty set is TRUE.

The plan was already right — LeftAnti ... Filter: oc.g > Int32(0) null_aware — so this is purely an execution fix. No optimizer change is involved.

What changes are included in this PR?

A NULL now makes NOT IN UNKNOWN only for the build rows whose correlation scope and residual filter keep that NULL, recorded per build row in a null-indices bitmap. Candidates come from a scope-map lookup when there are correlation keys and from a cross product otherwise, then pass the filter. Cost is proportional to the number of NULLs and is zero when the data has none; build rows already marked UNKNOWN are skipped.

Null-aware LeftAnti also accepts more than one join key, which the equality-correlated shape needs. RightAnti still requires exactly one.

What is the testing strategy for this PR?

The coverage landed in #25558. This PR flips the expectations it fixes — the diff in null_aware_anti_join.slt and the Q05–Q07 canaries is the behaviour change. datafusion/physical-plan/src/joins/hash_join/exec.rs also gains unit tests for the filter-only anti and mark paths at several batch sizes.

Are there any user-facing changes?

Correlated NOT IN with a residual filter returns correct results. Some shapes that failed to plan now run.

🤖 Generated with Claude Code

adriangb and others added 2 commits September 20, 2026 21:57
Adds sqllogictest and benchmark coverage for correlated `NOT IN`, pinned to
what DataFusion does today. Several of these expectations are wrong, and a few
shapes do not plan at all; each such block carries a note and a link to
apache#25336. The fixes flip them, so the
flip is visible in those diffs rather than buried in a large change.

sqllogictest, in `null_aware_anti_join.slt` and `null_aware_mark_join.slt`:
- correlated `NOT IN` with a non-equality correlation, which stays a residual
  join filter;
- a correlation that names only outer columns, so it cannot become an
  equi-join key;
- a constant value expression with and without a correlation;
- a subquery inside the `IN` value, both spellings of the outer `IN`;
- `IS NOT NULL` over a subquery predicate and a comparison between two marks,
  the contexts that can tell a NULL mark from a FALSE mark;
- plan pins for the joins the planner chooses in each case.

Benchmarks:
- Q09, a correlated non-negated `IN`. It must not use a null-aware join, a
  direction none of Q01-Q08 covers, and it passes today;
- correctness canaries on Q05-Q08, each comparing the `NOT IN` result with a
  reference that does not use `NOT IN`. All four currently disagree, so they
  are pinned to `false`.

Expected results verified with DuckDB and PostgreSQL.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…NOWN

A correlated `NOT IN` whose correlation is not an equi-join key leaves a
residual join filter. The null-aware hash join ignored it when deciding
whether a NULL on the subquery side makes `NOT IN` UNKNOWN, so a NULL that the
filter excludes still poisoned every outer row:

    CREATE TABLE oc(id INT, g INT) AS VALUES (1,5),(2,5),(3,0),(4,NULL),(NULL,5),(NULL,0);
    CREATE TABLE ic(id INT) AS VALUES (1),(NULL);
    SELECT id, g FROM oc WHERE oc.id NOT IN (SELECT ic.id FROM ic WHERE oc.g > 0);

returned no rows; DuckDB and PostgreSQL return three. The plan was already
correct, `LeftAnti ... Filter: oc.g > Int32(0) null_aware`, so this is purely
an execution fix.

A NULL now makes `NOT IN` UNKNOWN only for the build rows whose correlation
scope and residual filter keep that NULL, recorded per build row in a
null-indices bitmap. Candidates come from a scope-map lookup when there are
correlation keys and from a cross product otherwise, then pass the filter. The
cost is proportional to the number of NULLs and is zero when the data has
none; build rows already marked UNKNOWN are skipped.

Null-aware `LeftAnti` also accepts more than one join key now, which the
equality-correlated shape needs; `RightAnti` still requires exactly one.

Flips the affected expectations from the preceding commit, and the Q05-Q07
correctness canaries.

Closes apache#25336

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Sep 21, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.66197% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.42%. Comparing base (1e09a2a) to head (af07a2d).

Files with missing lines Patch % Lines
...fusion/physical-plan/src/joins/hash_join/stream.rs 93.40% 6 Missing and 7 partials ⚠️
...tafusion/physical-plan/src/joins/hash_join/exec.rs 94.18% 1 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #25559    +/-   ##
========================================
  Coverage   82.42%   82.42%            
========================================
  Files        1138     1138            
  Lines      435429   435602   +173     
  Branches   435429   435602   +173     
========================================
+ Hits       358889   359043   +154     
- Misses      54839    54851    +12     
- Partials    21701    21708     +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Optimizer rules physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong results: correlated NOT IN with a non-equality correlation returns no rows (null-aware LeftAnti join ignores the residual filter)

2 participants