fix: correlated NOT IN with a non-equality correlation returns wrong results

[adriangb]

Which issue does this PR close?

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

Rationale for this change

A correlated NOT IN subquery gives wrong results when the correlation is not an equality and the subquery column contains NULL. There is no error and no warning.

CREATE TABLE t1(id INT, z INT) AS VALUES (1,10), (2,20), (NULL,30), (4,40);
CREATE TABLE t2(id INT, z INT) AS VALUES (1,5), (NULL,50);

SELECT id FROM t1 WHERE id NOT IN (SELECT t2.id FROM t2 WHERE t2.z < t1.z) ORDER BY id;
-- main: (no rows)    this PR, DuckDB, PostgreSQL: 2, 4

SELECT id FROM t1 WHERE NOT (id IN (SELECT t2.id FROM t2 WHERE t2.z < t1.z)) OR id = 4 ORDER BY id;
-- main: 2, 4, NULL   this PR, DuckDB, PostgreSQL: 2, 4

The same gap also makes an equality-correlated NOT IN in a WHERE clause fail to plan:

SELECT id FROM t1 WHERE id NOT IN (SELECT t2.id FROM t2 WHERE t2.z = t1.z);
-- main: Error during planning: null_aware LeftAnti joins only support single column join key, got 2 columns

The fix sketch in the issue (turn off null_aware for a LeftAnti join that has a join filter) does not work. I tried it: the plain anti join ignores NULLs completely, so queries that are correct today start to return rows. For example, id NOT IN (SELECT t2.id FROM t2 WHERE t2.z > t1.z) must return no rows, and returns 1, 2, 4, NULL with that change.

What changes are included in this PR?

The hash join already had the right mechanism for correlated NOT IN mark joins with equality correlation keys: a per-build-row bitmap that records "this row's NOT IN is UNKNOWN". This PR uses that mechanism for every correlated null-aware join and makes it apply the join filter. The commits are split for review:

1. HashJoinExec: a null-aware LeftAnti or LeftMark join is correlated when it has correlation scope keys or a join filter. For a NULL value on either side, the join finds the candidate (build, probe) row pairs through the scope key hash map, or takes all pairs when there are no scope keys. The join filter then decides which pairs make a build row UNKNOWN. The LeftAnti final stage drops those rows. The extra work is only for rows that have a NULL value key, so it is zero when the data has no NULLs. JoinSelection swaps a null-aware LeftAnti only when it has a single key and no filter.
2. DecorrelatePredicateSubquery: a NOT IN mark join with a non-equality correlation is now planned as null-aware. Four EXPLAIN results in subquery.slt change: the mark join now shows null_aware, and in one of them the join is no longer swapped to RightMark, because null-aware mark joins are never swapped.
3. Tests.

What is the testing strategy for this PR?

• New sqllogictest cases in null_aware_anti_join.slt and null_aware_mark_join.slt: the queries from the issue, NULL outer values with empty and non-empty subquery results, equality plus non-equality correlation, a filter on the subquery value itself, the positive IN form, the mark column through IS NULL / IS TRUE / IS FALSE / NOT ... OR and directly in a SELECT list, and runs with batch_size = 1. I checked all expected results with DuckDB 1.5.2 and PostgreSQL 17.11. 14 of these cases fail on main.
• New HashJoinExec unit tests for a null-aware LeftAnti and LeftMark join that has a join filter and no scope keys, at all batch sizes.

Are there any user-facing changes?

Queries that returned wrong results now return correct results, and correlated NOT IN with an equality correlation in a WHERE clause no longer fails to plan. There are no public API changes.

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

❌ Patch coverage is 93.25397% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.92%. Comparing base (a0631ed) to head (e9a4ea3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...fusion/physical-plan/src/joins/hash_join/stream.rs	92.63%	5 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   #25339    +/-   ##
========================================
  Coverage   81.92%   81.92%            
========================================
  Files        1135     1135            
  Lines      427573   427693   +120     
  Branches   427573   427693   +120     
========================================
+ Hits       350279   350391   +112     
- Misses      56367    56370     +3     
- Partials    20927    20932     +5     

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

[kosiew]

@adriangb,

Thanks for working on this. The approach looks good to me, especially keeping the residual join filter involved when determining the per-build-row UNKNOWN state for correlated NOT IN. I also like the added coverage for both anti and mark joins.

I left one non-blocking performance suggestion below. Nothing that needs to hold up the PR.

[kosiew]

One potential performance concern here: when there are no scope keys, we evaluate the residual filter for every NULL build row × probe row pair. The symmetric path below does the same for NULL probe rows. For a nullable non-equality-correlated NOT IN, that could add quadratic work, including for build rows that have already been marked UNKNOWN.

Would it be worth adding a small bounded benchmark or targeted performance regression test for this path? As a follow-up optimization, we might also be able to skip build rows that are already marked UNKNOWN, although we'd need to be careful about volatile or erroring filter expressions.

[jayzhan211]

Thanks @adriangb , 2 non-blocking suggestions

[jayzhan211]

The case with no scope keys re-checks build rows that are already marked UNKNOWN

Without scope keys, case 2 evaluates the filter for every (build row × NULL probe row) pair in every probe batch, including build rows already set in null_indices_bitmap. Those bits never clear, so that work is wasted. 20K outer × 10K NULL inner with i.z < o.z spends 8.85s in join_time (debug build). Skip marked rows and stop once none are left (same for case 1 at :1466):

None => {
    let num_build_rows = left_data.batch().num_rows();
    for probe_rows in null_probe_rows.values().chunks(batch_size.max(1)) {
        let build_rows = {
            let bitmap = left_data.null_indices_bitmap().lock();
            UInt64Array::from_iter_values(
                (0..num_build_rows)
                    .filter(|i| !bitmap.get_bit(*i))
                    .map(|i| i as u64),
            )
        };
        if build_rows.is_empty() {
            break;
        }
        let probe_rows = UInt32Array::from(probe_rows.to_vec());
        for_each_cross_product(&build_rows, &probe_rows, batch_size, &mut mark)?;
    }
}

Fine to handle in a follow-up

[jayzhan211]

correlated = num_keys > 1 || has_filter assumes on[0] is the NOT IN value key. When the value has no outer columns, 1 = i.id is pushed into the subquery, so on[0] becomes the correlation key o.g = i.g, and a NULL o.g is marked UNKNOWN:

CREATE TABLE o(id INT, g INT, z INT) AS VALUES (1,1,10),(2,NULL,10),(3,2,10);
CREATE TABLE i(id INT, g INT, z INT) AS VALUES (1,1,5),(5,2,5),(NULL,3,5);
SELECT id FROM o WHERE 1 NOT IN (SELECT i.id FROM i WHERE i.g = o.g AND i.z < o.z);
-- expected 2, 3; returns 3

The form without AND i.z < o.z is also wrong and doesn't go through the new code, so this predates the PR. Fine as a follow-up: in build_join, only set null_aware when the in-predicate's outer side references a left column.

[adriangb]

Agreed, this is a bug in main:

main:  R1 (with i.z < o.z) → 1 row    R2 (equality only) → 1 row
PR:    R1                  → 1 row    R2                 → 1 row

[adriangb]

@jayzhan211 @kosiew I opened #25386 w/ benchmarks for this change. Could we merge that first so we can look at before/afterS?