fix: simplify IN lists containing NULL literals in EliminateFilter

datafusion/optimizer/src/eliminate_filter.rs

@@ -6,6 +6,7 @@
 
 use datafusion_common::tree_node::Transformed;
 use datafusion_common::{Result, ScalarValue};
+use datafusion_expr::expr::InList;
 use datafusion_expr::{EmptyRelation, Expr, Filter, LogicalPlan, Operator};
 use std::sync::Arc;
 
@@ -86,6 +87,10 @@ struct SimplifiedFilterPredicate {
 /// throughout an `AND` / `OR` tree because both operators are monotonic with
 /// respect to whether their result can be `TRUE`. It is not safe through `NOT`
 /// or arbitrary expressions, so those are deliberately treated as leaves.
+///
+/// The same equivalence lets `IN` lists drop their `NULL` members, and makes
+/// `NOT IN` lists that contain one reject every row. See
+/// [`simplify_filter_in_list`].
 fn simplify_filter_predicate(expr: Expr) -> SimplifiedFilterPredicate {
     match expr {
         Expr::Literal(ScalarValue::Boolean(Some(true)), _) => {
@@ -100,10 +105,57 @@ fn simplify_filter_predicate(expr: Expr) -> SimplifiedFilterPredicate {
         Expr::BinaryExpr(binary) if binary.op == Operator::Or => {
             simplify_filter_or(*binary.left, *binary.right)
         }
+        Expr::InList(in_list) => simplify_filter_in_list(in_list),
         expr => simplified(FilterPredicate::Expression(expr), false),
     }
 }
 
+fn is_null_literal(expr: &Expr) -> bool {
+    matches!(expr, Expr::Literal(scalar, _) if scalar.is_null())
+}
+
+/// Simplifies an `IN` list under filter semantics by dropping `NULL` members.
+///
+/// `x NOT IN (a, NULL)` is `FALSE` when `x` matches `a` and `NULL` otherwise, so
+/// it is never `TRUE` and rejects every row.
+///
+/// This is only sound under filter semantics; the same rewrite in a general
+/// expression context would change `SELECT 3 IN (1, NULL)` from `NULL` to `FALSE`.
+fn simplify_filter_in_list(in_list: InList) -> SimplifiedFilterPredicate {
+    let InList {
+        expr,
+        mut list,
+        negated,
+    } = in_list;
+
+    // `retain` drops the `NULL`s in place. Most lists have none, and that case
+    // then costs one pass and no allocation.
+    let len_before = list.len();
+    list.retain(|e| !is_null_literal(e));
+
+    if list.len() == len_before {
+        // No `NULL` members, so there is nothing to simplify.
+        return simplified(
+            FilterPredicate::Expression(Expr::InList(InList::new(expr, list, negated))),
+            false,
+        );
+    }
+
+    if negated || list.is_empty() {
+        // `NOT IN` with a `NULL` member, and `IN` over nothing but `NULL`s, are
+        // both `NULL` for every row that they do not reject outright.
+        return simplified(
+            FilterPredicate::RejectsAll(Expr::Literal(ScalarValue::Boolean(None), None)),
+            true,
+        );
+    }
+
+    simplified(
+        FilterPredicate::Expression(Expr::InList(InList::new(expr, list, negated))),
+        true,
+    )
+}
+
 fn simplify_filter_and(left: Expr, right: Expr) -> SimplifiedFilterPredicate {
     let left = simplify_filter_predicate(left);
     let right = simplify_filter_predicate(right);

datafusion/sqllogictest/test_files/eliminate_outer_join.slt

@@ -84,9 +84,9 @@ explain select * from t1 left join t2 on t1.a = t2.x where t2.x in (1, 2, null);
 ----
 logical_plan
 01)Inner Join: t1.a = t2.x
-02)--Filter: t1.a IN ([Int32(1), Int32(2), Int32(NULL)])
+02)--Filter: t1.a IN ([Int32(1), Int32(2)])
 03)----TableScan: t1 projection=[a, b, c]
-04)--Filter: t2.x IN ([Int32(1), Int32(2), Int32(NULL)])
+04)--Filter: t2.x IN ([Int32(1), Int32(2)])
 05)----TableScan: t2 projection=[x, y, z]
 
 query IITIIT rowsort

datafusion/sqllogictest/test_files/in_list.slt

@@ -1059,3 +1059,91 @@ ORDER BY n
 # Cleanup
 statement ok
 DROP TABLE in_list_case_items;
+
+
+##########
+# IN lists containing NULL literals, under filter semantics
+#
+# A NULL member can only turn FALSE into NULL, and a filter keeps only the rows
+# for which its predicate is TRUE, so NULL rejects a row just like FALSE:
+#   x IN (a, NULL)     keeps exactly the rows that x IN (a) keeps
+#   x IN (NULL)        keeps nothing
+#   x NOT IN (a, NULL) keeps nothing: it is FALSE on a match and NULL otherwise
+#
+# This holds only inside a filter. In a general expression context
+# SELECT 3 IN (1, NULL) must still be NULL, which is checked at the end.
+##########
+
+statement ok
+CREATE TABLE in_list_nulls (label VARCHAR, n INT) AS VALUES
+  ('one', 1),
+  ('eleven', 11),
+  ('other', 42);
+
+statement ok
+set datafusion.explain.logical_plan_only = true;
+
+# The NULL member is dropped from the list. The list is kept over
+# THRESHOLD_INLINE_INLIST members so that it stays an IN list instead of being
+# rewritten into a chain of ORs.
+query TT
+explain select label from in_list_nulls where n in (1, 11, 42, 7, NULL);
+----
+logical_plan
+01)Projection: in_list_nulls.label
+02)--Filter: in_list_nulls.n IN ([Int32(1), Int32(11), Int32(42), Int32(7)])
+03)----TableScan: in_list_nulls projection=[label, n]
+
+# A list of nothing but NULLs rejects every row.
+query TT
+explain select label from in_list_nulls where n in (NULL);
+----
+logical_plan EmptyRelation: rows=0
+
+# NOT IN with a NULL member is never TRUE, so it rejects every row.
+query TT
+explain select label from in_list_nulls where n not in (1, 11, NULL);
+----
+logical_plan EmptyRelation: rows=0
+
+# Lists without a NULL member are left alone.
+query TT
+explain select label from in_list_nulls where n in (1, 11, 42, 7);
+----
+logical_plan
+01)Projection: in_list_nulls.label
+02)--Filter: in_list_nulls.n IN ([Int32(1), Int32(11), Int32(42), Int32(7)])
+03)----TableScan: in_list_nulls projection=[label, n]
+
+statement ok
+set datafusion.explain.logical_plan_only = false;
+
+# The rewritten filters return the same rows as the originals.
+query T rowsort
+select label from in_list_nulls where n in (1, 11, 42, 7, NULL);
+----
+eleven
+one
+other
+
+query T
+select label from in_list_nulls where n in (NULL);
+----
+
+query T
+select label from in_list_nulls where n not in (1, 11, 42, 7, NULL);
+----
+
+# Outside a filter the NULL must still make the result NULL, not FALSE.
+query B
+select 3 in (1, NULL);
+----
+NULL
+
+query B
+select 1 in (1, NULL);
+----
+true
+
+statement ok
+DROP TABLE in_list_nulls;

datafusion/sqllogictest/test_files/simplify_expr.slt

@@ -296,10 +296,10 @@ explain select price from decimal_prices where cast(price as decimal(38,15)) in
 );
 ----
 logical_plan
-01)Filter: CAST(decimal_prices.price AS Decimal128(38, 15)) IN ([Decimal128(4.000000000000000,38,15), Decimal128(4.000000000000001,38,15), Decimal128(NULL,38,15)])
+01)Filter: CAST(decimal_prices.price AS Decimal128(38, 15)) IN ([Decimal128(4.000000000000000,38,15), Decimal128(4.000000000000001,38,15)])
 02)--TableScan: decimal_prices projection=[price]
 physical_plan
-01)FilterExec: CAST(price@0 AS Decimal128(38, 15)) IN (SET) ([4.000000000000000, 4.000000000000001, NULL])
+01)FilterExec: CAST(price@0 AS Decimal128(38, 15)) IN (SET) ([4.000000000000000, 4.000000000000001])
 02)--DataSourceExec: partitions=1, partition_sizes=[1]
 
 query TT