viirya commented on code in PR #19635:
URL: https://github.com/apache/datafusion/pull/19635#discussion_r2666837700


##########
datafusion/sqllogictest/test_files/null_aware_anti_join.slt:
##########
@@ -0,0 +1,414 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+
+#   http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#############
+## Null-Aware Anti Join Tests
+## Tests for automatic null-aware semantics in NOT IN subqueries
+#############
+
+statement ok
+CREATE TABLE outer_table(id INT, value TEXT) AS VALUES
+(1, 'a'),
+(2, 'b'),
+(3, 'c'),
+(4, 'd'),
+(NULL, 'e');
+
+statement ok
+CREATE TABLE inner_table_no_null(id INT, value TEXT) AS VALUES
+(2, 'x'),
+(4, 'y');
+
+statement ok
+CREATE TABLE inner_table_with_null(id INT, value TEXT) AS VALUES
+(2, 'x'),
+(NULL, 'y');
+
+#############
+## Test 1: NOT IN with no NULLs - should behave like regular anti join
+#############
+
+query IT rowsort
+SELECT * FROM outer_table WHERE id NOT IN (SELECT id FROM inner_table_no_null);
+----
+1 a
+3 c
+
+# Verify the plan uses LeftAnti join
+query TT
+EXPLAIN SELECT * FROM outer_table WHERE id NOT IN (SELECT id FROM 
inner_table_no_null);
+----
+logical_plan
+01)LeftAnti Join: outer_table.id = __correlated_sq_1.id
+02)--TableScan: outer_table projection=[id, value]
+03)--SubqueryAlias: __correlated_sq_1
+04)----TableScan: inner_table_no_null projection=[id]
+physical_plan
+01)HashJoinExec: mode=CollectLeft, join_type=LeftAnti, on=[(id@0, id@0)]
+02)--DataSourceExec: partitions=1, partition_sizes=[1]
+03)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+#############
+## Test 2: NOT IN with NULL in subquery - should return 0 rows (null-aware 
semantics)
+#############
+
+query IT rowsort
+SELECT * FROM outer_table WHERE id NOT IN (SELECT id FROM 
inner_table_with_null);
+----
+
+# Verify the result is empty even though there are rows in outer_table
+# that don't match the non-NULL value (2) in the subquery.
+# This is correct null-aware behavior: if subquery contains NULL, result is 
unknown.
+
+#############
+## Test 3: NOT IN with NULL in outer table but not in subquery
+## NULL rows from outer should not appear in output
+#############
+
+query IT rowsort
+SELECT * FROM outer_table WHERE id NOT IN (SELECT id FROM inner_table_no_null) 
AND id IS NOT NULL;
+----
+1 a
+3 c
+
+#############
+## Test 4: Test with all NULL subquery
+#############
+
+statement ok
+CREATE TABLE all_null_table(id INT) AS VALUES (NULL), (NULL);
+
+query IT rowsort
+SELECT * FROM outer_table WHERE id NOT IN (SELECT id FROM all_null_table);
+----
+
+#############
+## Test 5: Test with empty subquery - should return all rows
+#############
+
+statement ok
+CREATE TABLE empty_table(id INT, value TEXT);
+
+query IT rowsort
+SELECT * FROM outer_table WHERE id NOT IN (SELECT id FROM empty_table);
+----
+1 a
+2 b
+3 c
+4 d
+
+#############
+## Test 6: NOT IN with complex expression
+#############
+
+query IT rowsort
+SELECT * FROM outer_table WHERE id + 1 NOT IN (SELECT id FROM 
inner_table_no_null);
+----
+2 b
+4 d
+
+#############
+## Test 7: NOT IN with complex expression and NULL in subquery
+#############
+
+query IT rowsort
+SELECT * FROM outer_table WHERE id + 1 NOT IN (SELECT id FROM 
inner_table_with_null);
+----
+
+#############
+## Test 8: Multiple NOT IN conditions (AND)
+#############
+
+statement ok
+CREATE TABLE inner_table2(id INT) AS VALUES (1), (3);
+
+query IT rowsort
+SELECT * FROM outer_table
+WHERE id NOT IN (SELECT id FROM inner_table_no_null)
+  AND id NOT IN (SELECT id FROM inner_table2);
+----
+
+#############
+## Test 9: Multiple NOT IN conditions (OR)
+#############
+
+query IT rowsort
+SELECT * FROM outer_table
+WHERE id NOT IN (SELECT id FROM inner_table_no_null)
+   OR id NOT IN (SELECT id FROM inner_table2);
+----
+1 a
+2 b
+3 c
+4 d
+NULL e

Review Comment:
   The test expectation was indeed incorrect according to SQL semantics.
   
     The Problem
   
     Test 9 has the query:
     SELECT * FROM outer_table
     WHERE id NOT IN (SELECT id FROM inner_table_no_null)
        OR id NOT IN (SELECT id FROM inner_table2);
   
     For the NULL row:
     - NULL NOT IN (2, 4) = UNKNOWN
     - NULL NOT IN (1, 3) = UNKNOWN
     - UNKNOWN OR UNKNOWN = UNKNOWN → should be filtered out
   
     But the test was expecting (NULL, 'e') to be included, which is wrong.
   
     Root Cause
   
     When NOT IN subqueries appear in OR conditions, DataFusion uses RightMark 
joins instead of LeftAnti joins:
     1. Mark joins add a boolean "mark" column indicating whether each row had 
a match
     2. The filter then evaluates NOT mark OR NOT mark
     3. The problem: Mark joins treat NULL keys as non-matching (FALSE) instead 
of UNKNOWN
     4. This causes NOT FALSE OR NOT FALSE = TRUE, incorrectly including the 
NULL row
   
     Why This Happens
   
     Mark joins are designed to handle complex boolean expressions (like OR) by 
converting the subquery check into a boolean column. However, they don't 
implement null-aware semantics - the mark column is never NULL, even when it 
should be UNKNOWN due to NULL join keys.
   
     The Solution (For Now)
   
     The proper fix would be to implement null-aware support for mark joins, 
making the mark column nullable and setting it to NULL when join keys are NULL. 
However, this is a complex change that affects the core join implementation.
   
     For now, I've:
     1. Kept the test as-is (returning NULL row)
     2. Added detailed comments documenting this as a KNOWN LIMITATION
     3. Marked it as a TODO for future implementation
   
     This way, the limitation is clearly documented and users/developers are 
aware of the issue, while we can address it properly in a future enhancement.
   



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