fix: NOT IN (subquery) with a constant value ignores NULLs in the subquery - #25348
Merged
Merged
Conversation
…ery NULLs `<constant> NOT IN (<subquery>)` in a `WHERE` clause returned every row when the subquery produced a NULL. `3 NOT IN (1, NULL)` is UNKNOWN, so the clause must remove the row; DuckDB and PostgreSQL return no rows. The value expression `3` has no column reference, so `find_valid_equijoin_key_pair` rejects `Int64(3) = __correlated_sq_1.id` and it stays in the join filter. Two things then went wrong: * The filter references only the subquery side, so `push_down_filter` moved it into the subquery as `Filter: t2.id = 3`, dropping the NULL rows before the join could observe them. * The join was left without equi-join keys and was planned as a `NestedLoopJoinExec`, which has no null-aware implementation, so the `null_aware` flag was silently discarded. Three changes: * `DecorrelatePredicateSubquery` projects a constant value expression as a column of the outer input, so the predicate becomes a real equi-join key and the existing null-aware hash join handles it. The rewrite is limited to uncorrelated subqueries: a correlation predicate would be a second join key and null-aware hash joins accept only one. * `push_down_all_join` never pushes predicates into the right input of a null-aware join, matching the check `infer_join_predicates` already has. * The physical planner returns an error instead of building a keyless null-aware join that silently ignores the flag. Regression tests cover the anti-join and mark-join shapes, the controls from the report, a user column colliding with the projected value column, and the remaining unsupported case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCeMPyJNgAiFpGXaz5CxF3
adriangb
commented
Sep 15, 2026
Comment on lines
+1607
to
+1612
| // Only `HashJoinExec` implements null-aware semantics, and it | ||
| // needs equi-join keys to do so. Without them the join would be | ||
| // planned as a nested loop (or piecewise merge) join, which | ||
| // silently ignores the flag and returns wrong results for | ||
| // `NOT IN` over a nullable subquery. Fail loudly instead. | ||
| if *null_aware && join_on.is_empty() { |
Contributor
Author
There was a problem hiding this comment.
Not sure how I feel about this. I guess if a user has their own join implementation that supports this they need custom physical planning. It seems in line with the rest of the code in this module, so maybe it's okay.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25348 +/- ##
==========================================
- Coverage 81.92% 81.92% -0.01%
==========================================
Files 1135 1135
Lines 427772 427879 +107
Branches 427772 427879 +107
==========================================
+ Hits 350456 350537 +81
- Misses 56373 56377 +4
- Partials 20943 20965 +22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
adriangb
force-pushed
the
claude/eloquent-feynman-4fwikn
branch
from
September 15, 2026 22:21
6a3a84a to
f8bce4a
Compare
Contributor
Author
|
@AdamGS would you be open to reviewing this change? Thanks! |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 17, 2026
adriangb
added a commit
to pydantic/datafusion
that referenced
this pull request
Sep 18, 2026
…nt `IN` call `join_keys_may_be_null` takes the equi-join keys and the residual filter since the previous commit. The call that the constant-value projection of apache#25348 makes still passes the earlier three arguments, so `datafusion-optimizer` does not build. The value expression of a constant `IN` holds no column, so the equality is not an equi-join key. There is no key expression to ask for its nullability, and the column test on the whole filter is the only test available there. The call thus passes no keys and the whole filter as the residual, which is what the earlier three-argument version did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
haohuaijin
pushed a commit
to haohuaijin/arrow-datafusion
that referenced
this pull request
Sep 19, 2026
…ubquery (apache#25348) ## Which issue does this PR close? - Closes apache#25340. A follow-up issue covers the remaining `NOT IN` problems. See **Follow-up work** at the end. ## Rationale for this change `3 NOT IN (1, NULL)` is UNKNOWN. A `WHERE` clause must remove the row. DataFusion kept every row. There was no error and no warning. ```sql CREATE TABLE t1(id INT) AS VALUES (1), (2); CREATE TABLE t2(id INT) AS VALUES (1), (NULL); SELECT id FROM t1 WHERE 3 NOT IN (SELECT id FROM t2) ORDER BY id; ``` | | result | |---|---| | DataFusion (before) | `1, 2` ❌ | | DataFusion (after) | *(no rows)* ✅ | | DuckDB 1.5.2 | *(no rows)* | | PostgreSQL 17.11 | *(no rows)* | The subquery gives `{1, NULL}`. The value `3` is not `NULL`, but `3` is also not known to be absent. Therefore the answer is UNKNOWN for every row of `t1`. The same expression in a `SELECT` list was already correct. Only the `WHERE` clause was wrong. ### Why it happened The value `3` holds no column. Therefore it could not become a join key. It stayed as a join filter. Two problems followed. ``` BEFORE AFTER WHERE 3 NOT IN (SELECT id FROM t2) WHERE 3 NOT IN (SELECT id FROM t2) │ │ ▼ ▼ anti join, no key anti join, key = (3, t2.id) filter: 3 = t2.id no filter │ │ ├─ (1) the filter moves into the ├─ (1) nothing to move │ subquery: WHERE t2.id = 3 │ │ the NULL row disappears │ │ │ └─ (2) no key, so the join becomes a └─ (2) the join has a key, so it nested loop join, which cannot stays a hash join, which do null-aware work. The flag is does null-aware work dropped without an error correctly │ │ ▼ ▼ 1, 2 ❌ (no rows) ✅ ``` ## What changes are included in this PR? Three changes. **1. Make the constant a join key** (`datafusion/optimizer/src/decorrelate_predicate_subquery.rs`) Add the constant to the outer side as a column. The comparison then becomes a true equality of two columns, so the existing null-aware hash join does the work. ``` Before: LeftAnti Join: Filter: Int64(3) = __correlated_sq_1.id null_aware → NestedLoopJoinExec (the null_aware flag is lost) After: LeftAnti Join: __correlated_sq_1_value = __correlated_sq_1.id null_aware Projection: t1.id, Int64(3) AS __correlated_sq_1_value → HashJoinExec ... null_aware ``` This applies only to uncorrelated subqueries. A correlated subquery needs a second key, and a null-aware anti join accepts only one key. **2. Keep the filter out of the subquery** (`datafusion/optimizer/src/push_down_filter.rs`) Do not move a predicate into the subquery side of a null-aware join. The NULLs must reach the join. `infer_join_predicates` has the same rule already. **3. Report an error instead of a wrong answer** (`datafusion/core/src/physical_planner.rs`) Only a hash join can do null-aware work, and it needs a key. If a null-aware join has no key, report an error. Do not build a nested loop join that gives wrong results without a warning. ## What is the testing strategy for this PR? New `sqllogictest` cases in `datafusion/sqllogictest/test_files/null_aware_anti_join.slt` and `null_aware_mark_join.slt`. They cover: - the four queries of the issue, and its controls; - a subquery with no NULL, which must keep all rows; - the `OR` and `IS NULL` forms, which use a mark join; - a user column with the same name as the new column, which must not be ambiguous; - the one shape that is not supported, which must report an error. New unit tests in `decorrelate_predicate_subquery.rs` and `push_down_filter.rs` hold the new plans. Results: | check | result | |---|---| | full `sqllogictest` suite | pass, and **no plan changes** anywhere else | | `datafusion-optimizer` unit tests | 853 pass | | extended workspace suite | 10,784 tests pass, 0 fail, 66 crates | | `cargo fmt --all` | clean | | `cargo clippy` | clean for the changed crates | The extended workspace figure comes from an earlier run of the same commit on a slightly older base. The full `sqllogictest` suite and the optimizer tests were re-run after the rebase onto current `main`. ## Are there any user-facing changes? Yes. Two. **1. `<constant> NOT IN (<subquery>)` in a `WHERE` clause now gives correct results.** This is the fix. **2. One shape now reports an error.** A constant with a correlation that is not an equality: ```sql SELECT id FROM t1 WHERE 3 NOT IN (SELECT id FROM t2 WHERE t2.g > t1.g); ``` ``` Error during planning: null_aware LeftAnti join requires equi-join keys, but the join has none ``` This query gave wrong results before. No correct result is lost. An error is better than a wrong answer that a user cannot see. There are no API changes. ## Follow-up work This PR makes every **uncorrelated** `NOT IN` correct. **Correlated** `NOT IN` still has problems. They come from the join operator, not from the code this PR changes. ``` NOT IN (subquery) │ ├── no outer column ──────────────▶ CORRECT (this PR) │ └── reads an outer column ├── equality condition ───────▶ WRONG or ERROR (follow-up, parts 1 and 2) └── other condition ──────────▶ WRONG (follow-up, part 3) ``` A test matrix of 18 query shapes gives these totals: | | wrong or error | |---|---| | before this PR | 13 of 18 | | after this PR | 10 of 18 | The 10 remaining shapes are all correlated. They fall into three parts: 1. A null-aware anti join accepts only one key, but a correlated `NOT IN` needs two. 2. A constant does not become a key when the subquery is correlated. 3. A null-aware join looks for NULLs only in the key, not in a leftover filter. Part 3 must come first, as an error. A prototype showed that part 2 alone turns a clear error into a silent wrong answer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VCeMPyJNgAiFpGXaz5CxF3 Co-authored-by: Claude <noreply@anthropic.com>
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?
A follow-up issue covers the remaining
NOT INproblems. See Follow-up work at the end.Rationale for this change
3 NOT IN (1, NULL)is UNKNOWN. AWHEREclause must remove the row. DataFusion kept every row. There was no error and no warning.1, 2❌The subquery gives
{1, NULL}. The value3is notNULL, but3is also not known to be absent. Therefore the answer is UNKNOWN for every row oft1.The same expression in a
SELECTlist was already correct. Only theWHEREclause was wrong.Why it happened
The value
3holds no column. Therefore it could not become a join key. It stayed as a join filter. Two problems followed.What changes are included in this PR?
Three changes.
1. Make the constant a join key (
datafusion/optimizer/src/decorrelate_predicate_subquery.rs)Add the constant to the outer side as a column. The comparison then becomes a true equality of two columns, so the existing null-aware hash join does the work.
This applies only to uncorrelated subqueries. A correlated subquery needs a second key, and a null-aware anti join accepts only one key.
2. Keep the filter out of the subquery (
datafusion/optimizer/src/push_down_filter.rs)Do not move a predicate into the subquery side of a null-aware join. The NULLs must reach the join.
infer_join_predicateshas the same rule already.3. Report an error instead of a wrong answer (
datafusion/core/src/physical_planner.rs)Only a hash join can do null-aware work, and it needs a key. If a null-aware join has no key, report an error. Do not build a nested loop join that gives wrong results without a warning.
What is the testing strategy for this PR?
New
sqllogictestcases indatafusion/sqllogictest/test_files/null_aware_anti_join.sltandnull_aware_mark_join.slt. They cover:ORandIS NULLforms, which use a mark join;New unit tests in
decorrelate_predicate_subquery.rsandpush_down_filter.rshold the new plans.Results:
sqllogictestsuitedatafusion-optimizerunit testscargo fmt --allcargo clippyThe extended workspace figure comes from an earlier run of the same commit on a slightly older base. The full
sqllogictestsuite and the optimizer tests were re-run after the rebase onto currentmain.Are there any user-facing changes?
Yes. Two.
1.
<constant> NOT IN (<subquery>)in aWHEREclause now gives correct results. This is the fix.2. One shape now reports an error. A constant with a correlation that is not an equality:
This query gave wrong results before. No correct result is lost. An error is better than a wrong answer that a user cannot see.
There are no API changes.
Follow-up work
This PR makes every uncorrelated
NOT INcorrect. CorrelatedNOT INstill has problems. They come from the join operator, not from the code this PR changes.A test matrix of 18 query shapes gives these totals:
The 10 remaining shapes are all correlated. They fall into three parts:
NOT INneeds two.Part 3 must come first, as an error. A prototype showed that part 2 alone turns a clear error into a silent wrong answer.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VCeMPyJNgAiFpGXaz5CxF3