adriangb opened a new issue, #25336:
URL: https://github.com/apache/datafusion/issues/25336

   ### Describe the bug
   
   A correlated `NOT IN` subquery gives wrong results when the correlation is a 
non-equality predicate.
   
   DataFusion drops every row. The correct answer has rows. DuckDB 1.5.2 and 
PostgreSQL 17.11 both give the correct answer.
   
   The optimizer rewrites the query to a `null_aware` `LeftAnti` hash join that 
keeps the correlated predicate as a residual join filter. At run time, a NULL 
key on the subquery side makes the join emit nothing. The residual filter is 
never applied to that NULL row. In this example the residual filter removes the 
NULL row for every outer row, so the NULL must not affect the result.
   
   There is a second symptom with the same cause. When the `IN` predicate is 
inside a disjunction, the planner uses a `LeftMark` join. That join is 
correctly not `null_aware` when a residual filter is present, but the non 
null-aware fallback then gives `false` instead of UNKNOWN for a NULL key. `NOT 
false` is `true`, so an extra NULL row appears in the output.
   
   This is a silent wrong-results bug. There is no error and no warning.
   
   Tested on commit `add66e424f` (`datafusion-cli` 55.1.0, release build).
   
   ### To Reproduce
   
   DDL:
   
   ```sql
   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);
   ```
   
   Queries:
   
   ```sql
   -- Q1
   SELECT id FROM t1 WHERE id NOT IN (SELECT t2.id FROM t2 WHERE t2.z < t1.z) 
ORDER BY id;
   
   -- Q2
   SELECT id FROM t1 WHERE NOT (id IN (SELECT t2.id FROM t2 WHERE t2.z < t1.z)) 
ORDER BY id;
   
   -- Q3
   SELECT id FROM t1 WHERE NOT (id IN (SELECT t2.id FROM t2 WHERE t2.z < t1.z)) 
OR id = 4 ORDER BY id;
   
   -- CTL (control, the positive form of Q1)
   SELECT id FROM t1 WHERE id IN (SELECT t2.id FROM t2 WHERE t2.z < t1.z) ORDER 
BY id;
   
   -- Q4 (uncorrelated residual, NULL row removed by the residual)
   SELECT id FROM t1 WHERE id NOT IN (SELECT t2.id FROM t2 WHERE t2.z < 40) 
ORDER BY id;
   
   -- Q5 (uncorrelated residual, NULL row kept by the residual)
   SELECT id FROM t1 WHERE id NOT IN (SELECT t2.id FROM t2 WHERE t2.z < 100) 
ORDER BY id;
   
   -- Q6 (correlated residual, NULL row kept for every outer row)
   SELECT id FROM t1 WHERE id NOT IN (SELECT t2.id FROM t2 WHERE t2.z > t1.z) 
ORDER BY id;
   ```
   
   Results:
   
   | Query | DataFusion `add66e424f` | DuckDB 1.5.2 | PostgreSQL 17.11 | 
Correct |
   | --- | --- | --- | --- | --- |
   | Q1 | (no rows) | 2, 4 | 2, 4 | 2, 4 |
   | Q2 | (no rows) | 2, 4 | 2, 4 | 2, 4 |
   | Q3 | 2, 4, NULL | 2, 4 | 2, 4 | 2, 4 |
   | CTL | 1 | 1 | 1 | 1 |
   | Q4 | 2, 4 | 2, 4 | 2, 4 | 2, 4 |
   | Q5 | (no rows) | (no rows) | (no rows) | (no rows) |
   | Q6 | (no rows) | (no rows) | (no rows) | (no rows) |
   
   Q1, Q2 and Q3 are wrong. The control and Q4, Q5 and Q6 are correct.
   
   Q5 and Q6 show that the null-aware behaviour is correct when the NULL row 
truly is a member of the subquery result. Q4 shows that an uncorrelated 
residual is safe, because the optimizer pushes it into the subquery and no 
residual join filter remains.
   
   Manual check of Q1 for `id = 2`. The subquery for `t1.z = 20` is `SELECT 
t2.id FROM t2 WHERE t2.z < 20`, which is `{1}`. `2 NOT IN {1}` is `true`, so 
`id = 2` must be in the output. The maximum `t1.z` is 40, so the row `t2.id = 
NULL, t2.z = 50` never satisfies `t2.z < t1.z`. That NULL is not a member of 
the subquery result for any outer row.
   
   ### Expected behavior
   
   Q1 and Q2 return `2` and `4`. Q3 returns `2` and `4`.
   
   ### Additional context
   
   `EXPLAIN` for Q1 on `add66e424f`:
   
   ```
   logical_plan
   Projection: t1.id
     LeftAnti Join: t1.id = __correlated_sq_1.id Filter: __correlated_sq_1.z < 
t1.z null_aware
       TableScan: t1 projection=[id, z]
       SubqueryAlias: __correlated_sq_1
         TableScan: t2 projection=[id, z]
   physical_plan
   HashJoinExec: mode=CollectLeft, join_type=LeftAnti, on=[(id@0, id@0)], 
filter=z@1 < z@0, projection=[id@0], null_aware
     DataSourceExec: partitions=1, partition_sizes=[1]
     DataSourceExec: partitions=1, partition_sizes=[1]
   ```
   
   `EXPLAIN` for Q3 on `add66e424f`:
   
   ```
   logical_plan
   Projection: t1.id
     Filter: NOT __correlated_sq_1.mark OR t1.id = Int32(4)
       Projection: t1.id, __correlated_sq_1.mark
         LeftMark Join: t1.id = __correlated_sq_1.id Filter: 
__correlated_sq_1.z < t1.z
           TableScan: t1 projection=[id, z]
           SubqueryAlias: __correlated_sq_1
             TableScan: t2 projection=[id, z]
   ```
   
   #### Suspected root cause
   
   The `null_aware` decision is made in `build_join` in 
`datafusion/optimizer/src/decorrelate_predicate_subquery.rs`.
   
   The `LeftMark` path at line 452 first checks that the join predicate has no 
residual part. It calls `split_eq_and_noneq_join_predicate` and requires 
`residual_filter.is_none()`:
   
   ```rust
   let mark_filter_is_hashable_only =
       if join_type == JoinType::LeftMark && in_predicate_opt.is_some() {
           let (_, residual_filter) = split_eq_and_noneq_join_predicate(...)?;
           residual_filter.is_none()
       } else {
           false
       };
   ```
   
   The `LeftAnti` path at line 504 has no such check. It only tests key 
nullability:
   
   ```rust
   let null_aware = join_type == JoinType::LeftAnti
       && in_predicate_opt.is_some()
       && join_keys_may_be_null(&join_filter, left.schema(), 
sub_query_alias.schema())?;
   ```
   
   So a `LeftAnti` join keeps `null_aware` even when the correlated predicate 
stays as a residual join filter.
   
   The execution side cannot handle that combination. In 
`datafusion/physical-plan/src/joins/hash_join/stream.rs`, 
`null_aware_skip_probe_batch` at line 1217 looks only at the join key column of 
the probe batch:
   
   ```rust
   let probe_key_column = &state.values[0];
   let probe_has_null = match mode {
       NullAwareMode::LeftAnti if !has_filter => {
           probe_key_column.logical_null_count() > 0
       }
       _ => probe_key_column.null_count() > 0,
   };
   left_data.record_probe_batch(state.batch.num_rows() > 0, probe_has_null);
   mode == NullAwareMode::LeftAnti && left_data.probe_side_has_null_hint()
   ```
   
   The `has_filter` flag only selects `logical_null_count` or `null_count`. The 
residual filter itself is never evaluated. A NULL in `t2.id` therefore sets the 
shared "probe side has NULL" flag, and the whole probe batch is skipped.
   
   `null_aware_left_anti_final_indices` at line 1275 then drops everything:
   
   ```rust
   if probe_summary.has_null {
       return (UInt64Array::new_null(0), UInt32Array::new_null(0));
   }
   ```
   
   This is correct for a plain `NOT IN` with no residual, because one NULL in 
the subquery result makes the predicate UNKNOWN for every outer row. It is 
wrong with a residual filter, because the residual decides per outer row 
whether the NULL row is in the subquery result at all.
   
   The `LeftAnti` `null_aware` decision in the decorrelation rule was added in 
commit `4c67d02089`, from https://github.com/apache/datafusion/pull/19635. The 
`LeftMark` residual check was added later, in commit `bed9dcd548`, from 
https://github.com/apache/datafusion/pull/21585. The residual guard was not 
applied to the `LeftAnti` path at that time.
   
   #### Fix sketch
   
   The small fix is to gate the `LeftAnti` `null_aware` decision on the same 
condition as the `LeftMark` path. Compute `split_eq_and_noneq_join_predicate` 
for the anti case too, and set `null_aware` only when there is no residual 
filter. This makes Q1 and Q2 fall back to a plain anti join plus the existing 
correct handling, at the cost of the null-aware optimization for correlated 
`NOT IN` with a non-equality correlation. The alternative and larger fix is to 
make the null-aware execution residual-aware, so that a probe row with a NULL 
key only poisons the outer rows for which the residual filter is true. That 
needs the per-outer-row evaluation that the current shared-flag design does not 
have. Q3 needs a separate change on the `LeftMark` side, because the non 
null-aware mark column gives `false` where SQL requires UNKNOWN.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to