adriangb commented on code in PR #25338:
URL: https://github.com/apache/datafusion/pull/25338#discussion_r4048977897
##########
datafusion/optimizer/src/decorrelate_predicate_subquery.rs:
##########
@@ -634,11 +755,40 @@ fn build_join(
// - NOT EXISTS: Uses two-valued logic, regular anti join is correct
// We can distinguish them: NOT IN has in_predicate_opt, NOT EXISTS does
not
//
- // Additionally, if the join keys are non-nullable on both sides, we don't
need
- // null-aware semantics because NULLs cannot exist in the data.
- 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())?;
+ // Additionally, if no join key can be NULL on either side, we don't need
+ // null-aware semantics because NULLs cannot exist in the keys.
+ let null_aware = if join_type == JoinType::LeftAnti &&
in_predicate_opt.is_some() {
+ let (equijoin_keys, residual_filter) =
split_eq_and_noneq_join_predicate(
+ join_filter.clone(),
+ left.schema(),
+ sub_query_alias.schema(),
+ )?;
+ if equijoin_keys.len() > 1 {
+ // A null-aware `LeftAnti` hash join supports one key only. A
+ // correlated `NOT IN` has two or more keys (the value and the
+ // correlation), so keep the column test on the whole filter here.
+ // A function key such as `upper(s)` over a non-nullable column
+ // then does not make the join null-aware and fail to plan. The
+ // column test misses a NULL that only the key expression makes,
+ // as in `NULLIF(id, 1)`: see
+ // https://github.com/apache/datafusion/issues/25347.
+ join_keys_may_be_null(
+ &[],
+ Some(&join_filter),
+ left.schema(),
+ sub_query_alias.schema(),
+ )?
+ } else {
+ join_keys_may_be_null(
+ &equijoin_keys,
+ residual_filter.as_ref(),
+ left.schema(),
+ sub_query_alias.schema(),
+ )?
+ }
Review Comment:
Good catch, and correct. Fixed in 3af87370c1.
I reproduced it on this branch with the tables from your example:
```sql
CREATE TABLE ra(k INT NOT NULL, z INT NOT NULL) AS VALUES (1, 10), (2, 20);
CREATE TABLE rb(k INT NOT NULL, z INT NOT NULL) AS VALUES (5, 50);
SELECT k FROM ra WHERE NULLIF(ra.k, 1) NOT IN (SELECT rb.k FROM rb WHERE
rb.z < ra.z);
-- correct, and `main`: 1, 2. this branch before the fix: 2
```
The plan was `LeftAnti Join: nullif(...) = rb.k Filter: rb.z < ra.z
null_aware`, and the executor dropped the NULL key row on a globally non-empty
probe side although the correlated subquery result for that row is empty.
The gate is now `equijoin_keys.len() > 1 || residual_filter.is_some()`, so a
residual keeps the column test, as more than one key already did.
`subquery_projection.slt` pins both directions of the shape: the empty-subquery
case, which is now correct, and the non-empty one, which keeps `main`'s wrong
row and needs #25336.
--
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]
