Fix incorrect results for NOT IN subqueries with nulls

[msirek]

Which issue does this PR close?

Closes #8241.

Rationale for this change

NOT IN queries executed as antijoin did not honor three-valued boolean logic.
The equality predicate built for antijoin, such as 1 = my_tab.a for the following
query is never true, so would previously make the filter appear to be qualified:

select 'qualified filter' where 1 NOT IN (select * from (values (3), (null)) my_tab(a));

The above NOT IN expression should evaluate the same as the expression 1 != 3 AND 1 != null,
which evaluates to null (which effectively is the same as false when used as a filter or join predicate).

What changes are included in this PR?

The code which decorrelates a NOT IN subquery into an antijoin with an equality condition
between the left and right expressions of the NOT IN predicate is updated so that where
previously the condition left_expression = right_expression was always built, we now build
left_expression = right_expression IS NOT FALSE whenever the left expression or right
expression is nullable.

Are these changes tested?

• unit tests for nullable left/right expressions of a NOT IN subquery predicate
• sqllogictests for nullable left/right expressions of a NOT IN subquery predicate

Are there any user-facing changes?

No

[yukkit]

It works for me.

[yukkit]

👍 beautiful unit tests

[jonahgao]

Could we use field_with_unqualified_name here? The unqualified name can be cloned from right_col.name.
So that there is no need to create unqualified_right_col, and we do not need to modify the create_col_from_scalar_expr function.

[msirek]

Thanks! I've made the suggested change.

[alamb]

Thank you @msirek -- I was going through old PRs and found this one that somehow slipped through.

I took the liberty of merging the code in the PR up from main.

The code is quite elegant (as all your previous PRs) -- but mind bending (as are many NULL related PRs). Some of the plans look like they have regressed and are no longer using HashJoin with equality predicates.

Perhaps we can use https://docs.rs/datafusion/latest/datafusion/physical_plan/joins/struct.HashJoinExec.html#method.null_equals_null or similar to mark the joins correctly and still use eqi-joins but with different null matching semantics

cc @jackwener

[alamb]

It seems like this is likely a regression in the query plan now because somehow the join can't use an eqijoin. Maybe we need to teach the join operator about this subtely, or set null_equal_null perhaps

[jackwener]

Maybe we should identify is distinct from, and then set the null_equal_null according to it.

[msirek]

I took the liberty of merging the code in the PR up from main.

Thank you!

Some of the plans look like they have regressed and are no longer using HashJoin with equality predicates.
Perhaps we can use https://docs.rs/datafusion/latest/datafusion/physical_plan/joins/struct.HashJoinExec.html#method.null_equals_null or similar to mark the joins correctly and still use eqi-joins but with different null matching semantics

The semantics of null_equals_null can't really be re-used here because it only works for comparing 2 nulls, as I understand it.
https://github.com/apache/arrow-datafusion/blob/d594e6257b34a5ad47112e26d41516aaeb19e6dd/datafusion/physical-plan/src/joins/hash_join.rs#L1081-L1084
https://arrow.apache.org/rust/arrow/compute/kernels/cmp/fn.not_distinct.html

For example, in this test case:

SELECT 'Found' WHERE 1 NOT IN (NULL, 2, 3);
0 rows in set.

The result of evaluating "1 IS NOT DISTINCT FROM NULL" is FALSE, the same as when comparing 1 with 2 and 3, so the NOT IN result would be TRUE (but should be NULL).

There is a good description of the needed semantics in null-aware antijoin, referenced in #8241.

For the general case:

SELECT * FROM t1 WHERE nullable_column1 NOT IN (SELECT nullable_column2 FROM t2);

The presence of any NULL in the subquery means the predicate should always evaluate to NULL for every outer row, so the antijoin would need to be taught to detect the presence of a NULL in the subquery and return no rows.
Similarly, the presence of NULL in an outer row could short circuit and avoid joining that row with the subquery (always evaluates to NULL).
For multicolumn NOT IN it would be more complicated.

If we don't want any plan regressions, a null-aware flavor of hash antijoin seems to be needed.

[jackwener]

Get it, I thought wrong in the past, thanks @msirek.
We indeed need NAAJ.

[github-actions]

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.