Skip to content

Commit edc936f

Browse files
adriangbclaude
andauthored
fix: NOT IN (subquery) with a constant value ignores NULLs in the subquery (#25348)
## Which issue does this PR close? - Closes #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>
1 parent c4f5a9e commit edc936f

5 files changed

Lines changed: 355 additions & 4 deletions

File tree

‎datafusion/core/src/physical_planner.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,6 +1604,17 @@ impl DefaultPhysicalPlanner {
16041604
&& session_state.config().repartition_joins()
16051605
&& !*null_aware;
16061606

1607+
// Only `HashJoinExec` implements null-aware semantics, and it
1608+
// needs equi-join keys to do so. Without them the join would be
1609+
// planned as a nested loop (or piecewise merge) join, which
1610+
// silently ignores the flag and returns wrong results for
1611+
// `NOT IN` over a nullable subquery. Fail loudly instead.
1612+
if *null_aware && join_on.is_empty() {
1613+
return plan_err!(
1614+
"null_aware {join_type} join requires equi-join keys, but the join has none"
1615+
);
1616+
}
1617+
16071618
// TODO: Allow PWMJ to deal with residual equijoin conditions
16081619
let join: Arc<dyn ExecutionPlan> = if join_on.is_empty() {
16091620
if join_filter.is_none() && *join_type == JoinType::Inner {

‎datafusion/optimizer/src/decorrelate_predicate_subquery.rs‎

Lines changed: 117 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,14 @@ fn build_join(
471471
replace_qualified_name(filter, &all_correlated_cols, &alias).map(Some)
472472
})?;
473473

474-
let join_filter = match (join_filter_opt, in_predicate_opt.cloned()) {
474+
// The outer value expression of an `IN`/`NOT IN` predicate whose join filter
475+
// is nothing but that predicate, recorded together with the subquery column
476+
// it is compared against and a name for the column it can be projected as.
477+
// Correlated subqueries are excluded on purpose: their correlation predicate
478+
// is a second join key, and null-aware hash joins accept only a single key.
479+
let mut in_value_expr = None;
480+
481+
let mut join_filter = match (join_filter_opt, in_predicate_opt.cloned()) {
475482
(
476483
Some(join_filter),
477484
Some(Expr::BinaryExpr(BinaryExpr {
@@ -493,14 +500,56 @@ fn build_join(
493500
right,
494501
})),
495502
) => {
503+
let value_name = format!("{alias}_value");
496504
let right_col = create_col_from_scalar_expr(&right, alias)?;
505+
let value = left.deref().clone();
506+
in_value_expr = Some((value.clone(), right_col.clone(), value_name));
497507

498-
Expr::eq(left.deref().clone(), Expr::Column(right_col))
508+
Expr::eq(value, Expr::Column(right_col))
499509
}
500510
(None, None) => lit(true),
501511
_ => return Ok(None),
502512
};
503513

514+
// `<constant> IN/NOT IN (<subquery>)`: the outer value expression holds no
515+
// column reference, so `<constant> = __correlated_sq.col` is not a valid
516+
// equi-join key (see `find_valid_equijoin_key_pair`) and stays in the join
517+
// filter. Two things then go wrong for a null-aware join: the filter is
518+
// right-only, so `push_down_filter` moves it into the subquery and drops the
519+
// very NULLs that make `NOT IN` UNKNOWN, and a join without equi-join keys
520+
// is planned as a nested loop join, which has no null-aware implementation.
521+
// Projecting the constant as a column of the outer side turns the predicate
522+
// into a real equi-join key so the null-aware hash join handles it.
523+
let mut projected_left = None;
524+
if let Some((value, right_col, mut value_name)) = in_value_expr
525+
&& value.column_refs().is_empty()
526+
&& matches!(join_type, JoinType::LeftAnti | JoinType::LeftMark)
527+
&& join_keys_may_be_null(&join_filter, left.schema(), sub_query_alias.schema())?
528+
{
529+
// The projected column is unqualified, so a left field that already has
530+
// this name — however unlikely — would make the reference ambiguous.
531+
let left_schema = left.schema();
532+
while left_schema.fields().iter().any(|f| f.name() == &value_name) {
533+
value_name.push('_');
534+
}
535+
let value_col = Column::new_unqualified(value_name);
536+
let projections = left_schema
537+
.columns()
538+
.into_iter()
539+
.map(Expr::from)
540+
.chain(std::iter::once(value.alias(value_col.name())))
541+
.collect::<Vec<_>>();
542+
projected_left = Some(
543+
LogicalPlanBuilder::from(left.clone())
544+
.project(projections)?
545+
.build()?,
546+
);
547+
// `in_value_expr` is only set when the `IN` equality is the whole join
548+
// filter, so it can simply be rebuilt against the projected column.
549+
join_filter = Expr::eq(Expr::Column(value_col), Expr::Column(right_col));
550+
}
551+
let left = projected_left.as_ref().unwrap_or(left);
552+
504553
if matches!(join_type, JoinType::LeftMark | JoinType::RightMark) {
505554
let right_schema = sub_query_alias.schema();
506555

@@ -1413,6 +1462,72 @@ mod tests {
14131462
)
14141463
}
14151464

1465+
/// A constant value expression has no column, so `Int32(3) = inner_t.id`
1466+
/// cannot be an equi-join key on its own. The rule projects the constant as
1467+
/// a column of the outer side; `ExtractEquijoinPredicate` (not run here)
1468+
/// then turns the filter into a real key for the null-aware hash join.
1469+
#[test]
1470+
fn constant_not_in_subquery_projects_value_as_join_key() -> Result<()> {
1471+
let outer_scan = nullable_scalar_mark_scan("outer_t")?;
1472+
let inner_scan = nullable_scalar_mark_scan("inner_t")?;
1473+
1474+
let subquery = Arc::new(
1475+
LogicalPlanBuilder::from(inner_scan)
1476+
.project(vec![col("inner_t.id")])?
1477+
.build()?,
1478+
);
1479+
1480+
let plan = LogicalPlanBuilder::from(outer_scan)
1481+
.filter(not_in_subquery(lit(3i32), subquery))?
1482+
.build()?;
1483+
1484+
assert_optimized_plan_equal!(
1485+
plan,
1486+
@r"
1487+
Projection: outer_t.id, outer_t.grp [id:Int32;N, grp:Int32;N]
1488+
LeftAnti Join: Filter: __correlated_sq_1_value = __correlated_sq_1.id null_aware [id:Int32;N, grp:Int32;N, __correlated_sq_1_value:Int32]
1489+
Projection: outer_t.id, outer_t.grp, Int32(3) AS __correlated_sq_1_value [id:Int32;N, grp:Int32;N, __correlated_sq_1_value:Int32]
1490+
TableScan: outer_t [id:Int32;N, grp:Int32;N]
1491+
SubqueryAlias: __correlated_sq_1 [id:Int32;N]
1492+
Projection: inner_t.id [id:Int32;N]
1493+
TableScan: inner_t [id:Int32;N, grp:Int32;N]
1494+
"
1495+
)
1496+
}
1497+
1498+
/// The same rewrite must not fire for a correlated subquery: the
1499+
/// correlation predicate is a second equi-join key, and null-aware hash
1500+
/// joins accept only one.
1501+
#[test]
1502+
fn constant_not_in_correlated_subquery_is_not_rewritten() -> Result<()> {
1503+
let outer_scan = nullable_scalar_mark_scan("outer_t")?;
1504+
let inner_scan = nullable_scalar_mark_scan("inner_t")?;
1505+
1506+
let subquery = Arc::new(
1507+
LogicalPlanBuilder::from(inner_scan)
1508+
.filter(
1509+
out_ref_col(DataType::Int32, "outer_t.grp").eq(col("inner_t.grp")),
1510+
)?
1511+
.project(vec![col("inner_t.id")])?
1512+
.build()?,
1513+
);
1514+
1515+
let plan = LogicalPlanBuilder::from(outer_scan)
1516+
.filter(not_in_subquery(lit(3i32), subquery))?
1517+
.build()?;
1518+
1519+
assert_optimized_plan_equal!(
1520+
plan,
1521+
@r"
1522+
LeftAnti Join: Filter: Int32(3) = __correlated_sq_1.id AND outer_t.grp = __correlated_sq_1.grp null_aware [id:Int32;N, grp:Int32;N]
1523+
TableScan: outer_t [id:Int32;N, grp:Int32;N]
1524+
SubqueryAlias: __correlated_sq_1 [id:Int32;N, grp:Int32;N]
1525+
Projection: inner_t.id, inner_t.grp [id:Int32;N, grp:Int32;N]
1526+
TableScan: inner_t [id:Int32;N, grp:Int32;N]
1527+
"
1528+
)
1529+
}
1530+
14161531
#[test]
14171532
fn correlated_not_in_mark_join_is_null_aware_for_hashable_filter() -> Result<()> {
14181533
let outer_scan = nullable_scalar_mark_scan("outer_t")?;

‎datafusion/optimizer/src/push_down_filter.rs‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,25 @@ fn push_down_all_join(
406406
) -> Result<Transformed<LogicalPlan>> {
407407
let is_inner_join = join.join_type == JoinType::Inner;
408408
// Get pushable predicates from current optimizer state
409-
let (left_preserved, right_preserved) = lr_is_preserved(join.join_type);
409+
let (left_preserved, mut right_preserved) = lr_is_preserved(join.join_type);
410+
let (on_left_preserved, mut on_right_preserved) = on_lr_is_preserved(join.join_type);
411+
412+
// Null-aware joins (e.g. `NOT IN` with a nullable subquery) implement SQL
413+
// three-valued logic: a NULL join key on the right/subquery side makes the
414+
// predicate UNKNOWN and empties the result. Anything pushed into the right
415+
// input runs before the join can observe those NULLs, so a null-rejecting
416+
// predicate would drop them and silently produce wrong results.
417+
// `infer_join_predicates` skips null-aware joins for the same reason.
418+
//
419+
// `on_right_preserved` is what actually matters here: it is what lets a
420+
// right-only join filter — the shape `<constant> NOT IN (<subquery>)`
421+
// produces — reach the subquery. `right_preserved` is already false for
422+
// every join type that can carry `null_aware` today, so clearing it is
423+
// defence in depth.
424+
if join.null_aware {
425+
right_preserved = false;
426+
on_right_preserved = false;
427+
}
410428

411429
// The predicates can be divided to three categories:
412430
// 1) can push through join to its children(left or right)
@@ -447,7 +465,6 @@ fn push_down_all_join(
447465
}
448466

449467
let mut on_filter_join_conditions = vec![];
450-
let (on_left_preserved, on_right_preserved) = on_lr_is_preserved(join.join_type);
451468
for on in on_filter {
452469
if on_left_preserved && checker.is_left_only(&on) {
453470
left_push.push(on)
@@ -3891,6 +3908,46 @@ mod tests {
38913908
)
38923909
}
38933910

3911+
/// Regression test for a null-aware LeftAnti join whose join filter only
3912+
/// references the subquery side, the shape produced by
3913+
/// `<constant> NOT IN (<subquery>)`. Pushing that filter into the right
3914+
/// input would drop the subquery's NULL rows before the join can observe
3915+
/// them, so `NOT IN` would wrongly evaluate to TRUE instead of UNKNOWN.
3916+
#[test]
3917+
fn null_aware_left_anti_join_keeps_right_only_join_filter() -> Result<()> {
3918+
let table_scan = test_table_scan_with_name("test1")?;
3919+
let left = LogicalPlanBuilder::from(table_scan)
3920+
.project(vec![col("a"), col("b")])?
3921+
.build()?;
3922+
let right_table_scan = test_table_scan_with_name("test2")?;
3923+
let right = LogicalPlanBuilder::from(right_table_scan)
3924+
.project(vec![col("a"), col("b")])?
3925+
.build()?;
3926+
let plan = LogicalPlanBuilder::from(left)
3927+
.join_detailed_with_options(
3928+
right,
3929+
JoinType::LeftAnti,
3930+
(Vec::<Column>::new(), Vec::<Column>::new()),
3931+
Some(lit(3u32).eq(col("test2.a"))),
3932+
datafusion_common::NullEquality::NullEqualsNothing,
3933+
true,
3934+
)?
3935+
.build()?;
3936+
3937+
// `UInt32(3) = test2.a` stays on the join: it must not become a
3938+
// `TableScan: test2, full_filters=[...]`.
3939+
assert_optimized_plan_equal!(
3940+
plan,
3941+
@r"
3942+
LeftAnti Join: Filter: UInt32(3) = test2.a null_aware
3943+
Projection: test1.a, test1.b
3944+
TableScan: test1
3945+
Projection: test2.a, test2.b
3946+
TableScan: test2
3947+
"
3948+
)
3949+
}
3950+
38943951
#[test]
38953952
fn left_anti_join_with_filters() -> Result<()> {
38963953
let table_scan = test_table_scan_with_name("test1")?;

‎datafusion/sqllogictest/test_files/null_aware_anti_join.slt‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,3 +564,105 @@ DROP TABLE nai_outer;
564564

565565
statement ok
566566
DROP TABLE nai_inner;
567+
568+
#############
569+
## Regression: constant (non-column) value expression in `NOT IN (subquery)`
570+
##
571+
## `3 NOT IN (1, NULL)` is UNKNOWN, so a `WHERE` clause must remove every row.
572+
## The value expression `3` contains no column, so on its own it cannot be an
573+
## equi-join key; decorrelation projects it as a column of the outer side so
574+
## the null-aware hash join gets a real key. Otherwise the predicate stays in
575+
## the join filter, where it is pushed into the subquery (dropping the NULL
576+
## rows before the join can observe them) and leaves a keyless join that no
577+
## null-aware operator can execute.
578+
#############
579+
580+
statement ok
581+
CREATE TABLE naconst_t1(id INT) AS VALUES (1), (2);
582+
583+
statement ok
584+
CREATE TABLE naconst_t2(id INT) AS VALUES (1), (NULL);
585+
586+
# Q1: expected no rows (subquery yields {1, NULL} => `3 NOT IN {1, NULL}` is UNKNOWN)
587+
query I
588+
SELECT id FROM naconst_t1 WHERE 3 NOT IN (SELECT id FROM naconst_t2) ORDER BY id;
589+
----
590+
591+
# Q2: `NOT (x IN (...))` is the same predicate spelled differently
592+
query I
593+
SELECT id FROM naconst_t1 WHERE NOT (3 IN (SELECT id FROM naconst_t2)) ORDER BY id;
594+
----
595+
596+
# C1: the constant is present in the subquery => FALSE, no rows (already correct)
597+
query I
598+
SELECT id FROM naconst_t1 WHERE 1 NOT IN (SELECT id FROM naconst_t2) ORDER BY id;
599+
----
600+
601+
# C2: no NULL in the subquery => plain TRUE, all rows survive
602+
query I
603+
SELECT id FROM naconst_t1 WHERE 3 NOT IN (SELECT id FROM naconst_t2 WHERE id IS NOT NULL) ORDER BY id;
604+
----
605+
1
606+
2
607+
608+
# C3: column value expression (an equi-join key exists) => already correct
609+
query I
610+
SELECT id FROM naconst_t1 WHERE id + 2 NOT IN (SELECT id FROM naconst_t2) ORDER BY id;
611+
----
612+
613+
# A constant that is absent from a NULL-free subquery still yields TRUE even
614+
# when other outer predicates are present.
615+
query I
616+
SELECT id FROM naconst_t1
617+
WHERE id > 1 AND 3 NOT IN (SELECT id FROM naconst_t2 WHERE id IS NOT NULL)
618+
ORDER BY id;
619+
----
620+
2
621+
622+
# The projected value column is named after the subquery alias. A user column
623+
# that happens to share that name must not make the reference ambiguous.
624+
statement ok
625+
CREATE TABLE naconst_clash("__correlated_sq_1_value" INT) AS VALUES (1), (2);
626+
627+
query I
628+
SELECT "__correlated_sq_1_value" FROM naconst_clash
629+
WHERE 3 NOT IN (SELECT id FROM naconst_t2)
630+
ORDER BY 1;
631+
----
632+
633+
query I
634+
SELECT "__correlated_sq_1_value" FROM naconst_clash
635+
WHERE 3 NOT IN (SELECT id FROM naconst_t2 WHERE id IS NOT NULL)
636+
ORDER BY 1;
637+
----
638+
1
639+
2
640+
641+
statement ok
642+
DROP TABLE naconst_clash;
643+
644+
# A constant value expression with a non-equality correlation leaves the
645+
# null-aware join without any equi-join key. Only `HashJoinExec` implements
646+
# null-aware semantics and it needs a key, so the planner reports the gap
647+
# instead of falling back to a nested loop join that ignores the NULLs and
648+
# silently returns wrong results.
649+
statement ok
650+
CREATE TABLE naconst_corr_t1(id INT, g INT) AS VALUES (1, 1), (2, 2);
651+
652+
statement ok
653+
CREATE TABLE naconst_corr_t2(id INT, g INT) AS VALUES (1, 1), (NULL, 2);
654+
655+
query error DataFusion error: Error during planning: null_aware LeftAnti join requires equi\-join keys, but the join has none
656+
SELECT id FROM naconst_corr_t1 WHERE 3 NOT IN (SELECT id FROM naconst_corr_t2 WHERE naconst_corr_t2.g > naconst_corr_t1.g);
657+
658+
statement ok
659+
DROP TABLE naconst_corr_t1;
660+
661+
statement ok
662+
DROP TABLE naconst_corr_t2;
663+
664+
statement ok
665+
DROP TABLE naconst_t1;
666+
667+
statement ok
668+
DROP TABLE naconst_t2;

0 commit comments

Comments
 (0)