Skip to content

fix: Decide NOT IN null-aware joins from expression nullability, not just column nullability - #25641

Closed
namanjain24-sudo wants to merge 4 commits into
apache:mainfrom
namanjain24-sudo:fix-not-in-null-aware-nullability
Closed

namanjain24-sudo wants to merge 4 commits into
apache:mainfrom
namanjain24-sudo:fix-not-in-null-aware-nullability

Conversation

@namanjain24-sudo

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

NOT IN (subquery) decides whether it needs null-aware join semantics by
checking if any column in the join filter is nullable. That misses a NULL
constant (NULL NOT IN (...)) or an expression that is nullable on its own
(CASE without ELSE, NULLIF) even when every column it reads is NOT NULL. In both cases a row that should be UNKNOWN (and dropped from the
result) is silently kept.

What changes are included in this PR?

join_keys_may_be_null in decorrelate_predicate_subquery.rs now checks the
nullability of each equality operand as an expression (via ExprSchemable),
not just the columns it references, falling back to "may be null" when that
can't be resolved to one side.

What is the testing strategy for this PR?

  • Two optimizer unit tests: a NULL constant against a NOT NULL subquery
    column, and a CASE expression over NOT NULL columns, both asserting the
    resulting join is null-aware.
  • New cases in null_aware_anti_join.slt covering both issues' tables,
    including the OR mark-join form and EXPLAIN assertions. Confirmed each
    new query fails on main and passes with this change.
  • cargo test -p datafusion-optimizer and the full default sqllogictest
    suite (521 files) pass. Checked the TPC-H Q16 plan (the only TPC-H query
    using this path) is unchanged.

Are there any user-facing changes?

NOT IN subqueries now return correct (fewer) rows for these two cases.
No public API changes.

@github-actions github-actions Bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) labels Sep 23, 2026
@codecov-commenter

codecov-commenter commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.49153% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.48%. Comparing base (04e6b37) to head (e85696b).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...on/optimizer/src/decorrelate_predicate_subquery.rs 69.49% 6 Missing and 30 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25641      +/-   ##
==========================================
- Coverage   82.49%   82.48%   -0.01%     
==========================================
  Files        1140     1140              
  Lines      438854   438965     +111     
  Branches   438854   438965     +111     
==========================================
+ Hits       362032   362094      +62     
- Misses      54952    54968      +16     
- Partials    21870    21903      +33     

☔ 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.

…just column nullability

closes: apache#25473
closes: apache#25474

`join_keys_may_be_null` only checked the schema nullability of the columns
a join filter references. A NULL constant or an expression that is nullable
even though every column it reads is NOT NULL (CASE without ELSE, NULLIF,
arithmetic with a NULL operand) was therefore never treated as needing
null-aware semantics, so a NOT IN predicate that should be UNKNOWN silently
kept the outer row instead.

Replace the column walk with a per-equality-conjunct nullability check that
evaluates each operand as an expression against whichever schema it
resolves to, falling back to nullable when that can't be determined.
The double-negation query was labeled as demonstrating a flip from false
to true. NOT UNKNOWN is UNKNOWN, not TRUE; the query was always empty and
is a control from the issue's own table, not evidence of the fix.
- A correlated NOT IN whose value equality is NOT NULL but whose
  correlation equality is nullable, checked second, exercises the loop
  continuing past the first conjunct.
- An operand referencing columns from both schemas exercises the
  conservative fallback in operand_may_be_null directly.

Verified each fails when the corresponding code path is broken.
@namanjain24-sudo
namanjain24-sudo force-pushed the fix-not-in-null-aware-nullability branch from 5fc19ab to 5509608 Compare September 24, 2026 21:22
apache#25560, merged after this branch, changed build_join to project a
constant IN/NOT IN value as a real column so it takes the on[0] key
position instead of the correlation predicate. That now also fires for
this test's correlated + null-aware case, which is exactly the
scenario apache#25560's own comment on build_join describes. The plan is
still null-aware with both equi-join conditions; only the constant's
representation changed.
@namanjain24-sudo

Copy link
Copy Markdown
Contributor Author

Bumping this — still open for review whenever someone has bandwidth.

@namanjain24-sudo

Copy link
Copy Markdown
Contributor Author

Superseded by #25338 — the new null-aware mark/anti join architecture already decides nullability per-expression, so both #25473 and #25474 are fixed on main. Verified with concrete queries; closing this.

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

Labels

optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

2 participants