From 2fe3253ab6ea6bd494ebe81a7bd1ead7b54e3f75 Mon Sep 17 00:00:00 2001 From: Ran Reichman Date: Tue, 6 Oct 2026 03:31:26 +0000 Subject: [PATCH 1/2] build the IN list static filter only from items whose leaves are all literals --- .../physical-expr/src/expressions/in_list.rs | 41 ++++++++++++++++++- .../sqllogictest/test_files/in_list.slt | 31 ++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/datafusion/physical-expr/src/expressions/in_list.rs b/datafusion/physical-expr/src/expressions/in_list.rs index 7a7cb25317c1..ec4519156b86 100644 --- a/datafusion/physical-expr/src/expressions/in_list.rs +++ b/datafusion/physical-expr/src/expressions/in_list.rs @@ -22,6 +22,7 @@ use std::hash::{Hash, Hasher}; use std::sync::Arc; use crate::PhysicalExpr; +use crate::expressions::Literal; use crate::physical_expr::physical_exprs_bag_equal; use arrow::array::*; @@ -31,6 +32,7 @@ use arrow::compute::kernels::boolean::{not, or_kleene}; use arrow::compute::kernels::cmp::eq as arrow_eq; use arrow::datatypes::*; +use datafusion_common::tree_node::TreeNode; use datafusion_common::{ DFSchema, Result, ScalarValue, assert_or_internal_err, exec_err, }; @@ -107,7 +109,7 @@ fn evaluate_list( /// Try to evaluate a list of expressions as constants. /// /// Returns: -/// - `Ok(Some(ArrayRef))` if all expressions are constants (can be evaluated on an empty RecordBatch) +/// - `Ok(Some(ArrayRef))` if all expressions are constants (built only from literals) /// - `Ok(None)` if the list contains non-constant expressions /// - `Err(...)` only for actual errors (not for non-constant expressions) /// @@ -117,6 +119,13 @@ fn try_evaluate_constant_list( list: &[Arc], schema: &Schema, ) -> Result> { + // An item can read a column yet return a scalar on an empty batch (a CASE whose + // WHEN matches all zero rows returns its THEN), so require literal leaves. + for expr in list { + if expr.exists(|e| Ok(e.children().is_empty() && !e.is::()))? { + return Ok(None); + } + } let batch = RecordBatch::new_empty(Arc::new(schema.clone())); match evaluate_list(list, &batch) { Ok(array) => Ok(Some(array)), @@ -539,9 +548,10 @@ pub fn in_list( #[cfg(test)] mod tests { use super::*; - use crate::expressions::{col, lit, try_cast}; + use crate::expressions::{binary, case, col, lit, try_cast}; use arrow::datatypes::{IntervalDayTime, IntervalMonthDayNano, i256}; use datafusion_common::plan_err; + use datafusion_expr::Operator; use datafusion_expr::type_coercion::binary::comparison_coercion; use datafusion_physical_expr_common::physical_expr::fmt_sql; use insta::assert_snapshot; @@ -3176,6 +3186,33 @@ mod tests { Ok(()) } + #[test] + fn test_in_list_case_item_reading_a_column() -> Result<()> { + // n IN (2, CASE WHEN d > 5 THEN 1 ELSE d END) + let schema = Schema::new(vec![ + Field::new("n", DataType::Int32, false), + Field::new("d", DataType::Int32, false), + ]); + let d = col("d", &schema)?; + let when = binary(Arc::clone(&d), Operator::Gt, lit(5i32), &schema)?; + let item = case(None, vec![(when, lit(1i32))], Some(d))?; + let expr = in_list(col("n", &schema)?, vec![lit(2i32), item], &false, &schema)?; + let batch = RecordBatch::try_new( + Arc::new(schema), + vec![ + Arc::new(Int32Array::from(vec![2, 4, 1])), + Arc::new(Int32Array::from(vec![0, 4, 3])), + ], + )?; + + let result = expr.evaluate(&batch)?.into_array(batch.num_rows())?; + assert_eq!( + as_boolean_array(&result), + &BooleanArray::from(vec![true, true, false]) + ); + Ok(()) + } + /// Helper: creates an InListExpr with `static_filter = None` /// to force the column-reference evaluation path. fn make_in_list_with_columns( diff --git a/datafusion/sqllogictest/test_files/in_list.slt b/datafusion/sqllogictest/test_files/in_list.slt index 3ff47afb643f..8fde8b11fe71 100644 --- a/datafusion/sqllogictest/test_files/in_list.slt +++ b/datafusion/sqllogictest/test_files/in_list.slt @@ -827,3 +827,34 @@ ORDER BY label match true false no_match false true nulls NULL NULL + +#### +## List items that read a column +#### + +statement ok +CREATE TABLE in_list_case_items (n INT, d INT) AS VALUES (2, 0), (10, 2), (5, 2), (7, 1); + +# A CASE item with a literal THEN is still evaluated per row, not once on an empty batch. +query IB +SELECT n, n IN (2, 100, 101, CASE WHEN d > 5 THEN 1 ELSE d END * 5) +FROM in_list_case_items +ORDER BY n +---- +2 true +5 false +7 false +10 true + +query I +SELECT n +FROM in_list_case_items +WHERE n IN (2, 100, 101, CASE WHEN d > 5 THEN 1 ELSE d END * 5) +ORDER BY n +---- +2 +10 + +# Cleanup +statement ok +DROP TABLE in_list_case_items; From 8b38069fcc4938c40f0f7c3ea7954109ec288c47 Mon Sep 17 00:00:00 2001 From: ranflarion Date: Fri, 9 Oct 2026 13:12:49 -0400 Subject: [PATCH 2/2] Update datafusion/physical-expr/src/expressions/in_list.rs add is_volatile_node Co-authored-by: Jay Zhan --- datafusion/physical-expr/src/expressions/in_list.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/datafusion/physical-expr/src/expressions/in_list.rs b/datafusion/physical-expr/src/expressions/in_list.rs index ec4519156b86..0b3f8067ae4b 100644 --- a/datafusion/physical-expr/src/expressions/in_list.rs +++ b/datafusion/physical-expr/src/expressions/in_list.rs @@ -122,7 +122,9 @@ fn try_evaluate_constant_list( // An item can read a column yet return a scalar on an empty batch (a CASE whose // WHEN matches all zero rows returns its THEN), so require literal leaves. for expr in list { - if expr.exists(|e| Ok(e.children().is_empty() && !e.is::()))? { + if expr.exists(|e| { + Ok(e.is_volatile_node() || (e.children().is_empty() && !e.is::())) + })? { return Ok(None); } }