You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/07 10:56:39 UTC
(datafusion) 01/01: fix: preserve null semantics in expression simplification (#24258)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24258-cc29ea12ae9c19640d98e43f5d3676aa3b334431
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 46bbf0db1accffb81822e4a7a374818b36b70def
Author: Morax <10...@users.noreply.github.com>
AuthorDate: Mon Sep 7 10:56:12 2026 +0000
fix: preserve null semantics in expression simplification (#24258)
## Which issue does this PR close?
- Closes #24246.
## Rationale for this change
Set algebra over `IN` / `NOT IN` expressions can erase SQL
three-valued-logic semantics when either the tested expression or a list
item is nullable. The `array_has` to `IN` rewrite has a similar
mismatch: an array containing NULL does not behave like an `IN` list
containing NULL.
Preserving those NULL branches is required in general expression
contexts. In a filter, however, both FALSE and NULL reject the row, so
the optimizer can still produce the strongest equivalent plan there.
## What changes are included in this PR?
- Keep `IN` / `NOT IN` set operations unchanged when list items may be
NULL, and preserve the NULL branch when an otherwise safe set operation
becomes a constant for a nullable tested expression.
- Avoid rewriting `array_has` to `IN` when a nullable array element
would change the result, while retaining the safe non-volatile
same-expression case.
- Extend `EliminateFilter` to simplify positive `AND` / `OR` trees
according to filter semantics. This restores plans such as
`EmptyRelation` for disjoint `IN` predicates while retaining correct
NULL behavior in projections. The simplification deliberately does not
cross `NOT` or arbitrary expressions.
- Add unit, runtime, and SQL logic regression coverage for NULL results
and the resulting optimized plans.
## Are these changes tested?
Yes. The following checks pass:
- `cargo clippy --all-targets --all-features -- -D warnings`
- `./dev/rust_lint.sh`
- `cargo test --profile=ci --test sqllogictests`
- `prlimit --nofile=65536:65536 -- env RUST_BACKTRACE=1 cargo test
--profile ci --exclude datafusion-examples --exclude
datafusion-benchmarks --exclude datafusion-cli --workspace --lib --tests
--bins --features
avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption`
## Are there any user-facing changes?
Yes. Expressions involving nullable inputs retain their SQL
three-valued-logic results, while equivalent filter predicates continue
to optimize away rejected rows. There are no public API changes.
---
datafusion/functions-nested/src/array_has.rs | 117 +++++++-
datafusion/optimizer/src/eliminate_filter.rs | 319 +++++++++++++++++++--
.../src/simplify_expressions/expr_simplifier.rs | 170 ++++++++++-
.../sqllogictest/test_files/array/array_has.slt | 7 +-
.../test_files/eliminate_outer_join.slt | 4 +-
datafusion/sqllogictest/test_files/predicates.slt | 27 +-
.../sqllogictest/test_files/simplify_expr.slt | 35 ++-
datafusion/sqllogictest/test_files/subquery.slt | 26 +-
8 files changed, 625 insertions(+), 80 deletions(-)
diff --git a/datafusion/functions-nested/src/array_has.rs b/datafusion/functions-nested/src/array_has.rs
index ee2b728bf7..156f9477f5 100644
--- a/datafusion/functions-nested/src/array_has.rs
+++ b/datafusion/functions-nested/src/array_has.rs
@@ -128,7 +128,7 @@ impl ScalarUDFImpl for ArrayHas {
fn simplify(
&self,
mut args: Vec<Expr>,
- _info: &datafusion_expr::simplify::SimplifyContext,
+ info: &datafusion_expr::simplify::SimplifyContext,
) -> Result<ExprSimplifyResult> {
let [haystack, needle] = take_function_args(self.name(), &mut args)?;
@@ -150,11 +150,19 @@ impl ScalarUDFImpl for ArrayHas {
ScalarValue::convert_array_to_scalar_vec(&scalar.to_array()?)
{
assert_eq!(scalar_values.len(), 1);
- let list = scalar_values
+ let values = scalar_values
.into_iter()
.flatten()
.flatten()
- .map(|v| Expr::Literal(v, None))
+ .collect::<Vec<_>>();
+
+ if values.iter().any(ScalarValue::is_null) {
+ return Ok(ExprSimplifyResult::Original(args));
+ }
+
+ let list = values
+ .into_iter()
+ .map(|value| Expr::Literal(value, None))
.collect();
return Ok(ExprSimplifyResult::Simplified(in_list(
@@ -167,12 +175,27 @@ impl ScalarUDFImpl for ArrayHas {
Expr::ScalarFunction(ScalarFunction { func, args })
if func == &make_array_udf() =>
{
- // make_array has a static set of arguments, so we can pull the arguments out from it
- return Ok(ExprSimplifyResult::Simplified(in_list(
- std::mem::take(needle),
- std::mem::take(args),
- false,
- )));
+ let mut has_unsafe_nullable_element = false;
+ for arg in args.iter() {
+ // `needle IN (needle)` preserves NULL semantics. A different
+ // nullable element does not: array_has([NULL], value) is false,
+ // while value IN (NULL) is NULL. Volatile expressions are
+ // evaluated separately, so syntactic equality is insufficient.
+ let safe_same_expr = arg == &*needle && !arg.is_volatile();
+ if info.nullable(arg)? && !safe_same_expr {
+ has_unsafe_nullable_element = true;
+ break;
+ }
+ }
+
+ if !has_unsafe_nullable_element {
+ // make_array has a static set of arguments, so we can pull the arguments out from it
+ return Ok(ExprSimplifyResult::Simplified(in_list(
+ std::mem::take(needle),
+ std::mem::take(args),
+ false,
+ )));
+ }
}
_ => {}
}
@@ -1197,10 +1220,10 @@ mod tests {
create_array,
},
buffer::OffsetBuffer,
- datatypes::{DataType, Field},
+ datatypes::{DataType, Field, Schema},
};
use datafusion_common::{
- DataFusionError, ScalarValue, config::ConfigOptions,
+ DataFusionError, ScalarValue, ToDFSchema, config::ConfigOptions,
utils::SingleRowListArrayBuilder,
};
use datafusion_expr::simplify::SimplifyContext;
@@ -1263,6 +1286,78 @@ mod tests {
);
}
+ #[test]
+ fn test_simplify_array_has_with_nullable_make_array_is_unchanged() {
+ let haystack = make_array(vec![col("c"), lit(1)]);
+ let needle = lit(2);
+ let original = vec![haystack, needle];
+ let context = SimplifyContext::builder()
+ .with_schema(
+ Schema::new(vec![Field::new("c", DataType::Int32, true)])
+ .to_dfschema_ref()
+ .unwrap(),
+ )
+ .build();
+
+ let result = ArrayHas::new()
+ .simplify(original.clone(), &context)
+ .unwrap();
+
+ let ExprSimplifyResult::Original(args) = result else {
+ panic!("Expected ExprSimplifyResult::Original")
+ };
+ assert_eq!(args, original);
+ }
+
+ #[test]
+ fn test_simplify_array_has_with_same_nullable_element() {
+ let needle = col("c");
+ let haystack = make_array(vec![needle.clone()]);
+ let context = SimplifyContext::builder()
+ .with_schema(
+ Schema::new(vec![Field::new("c", DataType::Int32, true)])
+ .to_dfschema_ref()
+ .unwrap(),
+ )
+ .build();
+
+ let result = ArrayHas::new()
+ .simplify(vec![haystack, needle.clone()], &context)
+ .unwrap();
+
+ let ExprSimplifyResult::Simplified(Expr::InList(in_list)) = result else {
+ panic!("Expected simplified expression")
+ };
+ assert_eq!(
+ in_list,
+ datafusion_expr::expr::InList {
+ expr: Box::new(needle.clone()),
+ list: vec![needle],
+ negated: false,
+ }
+ );
+ }
+
+ #[test]
+ fn test_simplify_array_has_with_null_list_item_is_unchanged() {
+ let haystack = lit(SingleRowListArrayBuilder::new(create_array!(
+ Int32,
+ [Some(1), None]
+ ))
+ .build_list_scalar());
+ let needle = lit(2);
+ let original = vec![haystack, needle];
+
+ let result = ArrayHas::new()
+ .simplify(original.clone(), &SimplifyContext::default())
+ .unwrap();
+
+ let ExprSimplifyResult::Original(args) = result else {
+ panic!("Expected ExprSimplifyResult::Original")
+ };
+ assert_eq!(args, original);
+ }
+
#[test]
fn test_simplify_array_has_with_null_to_null() {
let haystack = Expr::Literal(ScalarValue::Null, None);
diff --git a/datafusion/optimizer/src/eliminate_filter.rs b/datafusion/optimizer/src/eliminate_filter.rs
index 8be5fb0857..5df9c00347 100644
--- a/datafusion/optimizer/src/eliminate_filter.rs
+++ b/datafusion/optimizer/src/eliminate_filter.rs
@@ -15,18 +15,18 @@
// specific language governing permissions and limitations
// under the License.
-//! [`EliminateFilter`] replaces `where false` or `where null` with an empty relation.
+//! [`EliminateFilter`] removes filters that accept all or no input rows.
use datafusion_common::tree_node::Transformed;
use datafusion_common::{Result, ScalarValue};
-use datafusion_expr::{EmptyRelation, Expr, Filter, LogicalPlan};
+use datafusion_expr::{EmptyRelation, Expr, Filter, LogicalPlan, Operator};
use std::sync::Arc;
use crate::optimizer::ApplyOrder;
use crate::{OptimizerConfig, OptimizerRule};
-/// Optimization rule that eliminate the scalar value (true/false/null) filter
-/// with an [LogicalPlan::EmptyRelation]
+/// Optimization rule that eliminates filters whose predicates always accept or
+/// reject their input rows.
///
/// This saves time in planning and executing the query.
/// Note that this rule should be applied after simplify expressions optimizer rule.
@@ -58,27 +58,186 @@ impl OptimizerRule for EliminateFilter {
plan: LogicalPlan,
_config: &dyn OptimizerConfig,
) -> Result<Transformed<LogicalPlan>> {
- match plan {
- LogicalPlan::Filter(Filter {
- predicate: Expr::Literal(ScalarValue::Boolean(v), _),
- input,
- ..
- }) => match v {
- Some(true) => Ok(Transformed::yes(Arc::unwrap_or_clone(input))),
- Some(false) | None => Ok(Transformed::yes(LogicalPlan::EmptyRelation(
- EmptyRelation {
- produce_one_row: false,
- schema: Arc::clone(input.schema()),
- },
- ))),
- },
- _ => Ok(Transformed::no(plan)),
+ let LogicalPlan::Filter(Filter {
+ predicate, input, ..
+ }) = plan
+ else {
+ return Ok(Transformed::no(plan));
+ };
+
+ let simplified = simplify_filter_predicate(predicate);
+ match simplified.predicate {
+ FilterPredicate::AcceptsAll => {
+ Ok(Transformed::yes(Arc::unwrap_or_clone(input)))
+ }
+ FilterPredicate::RejectsAll(_) => Ok(Transformed::yes(
+ LogicalPlan::EmptyRelation(EmptyRelation {
+ produce_one_row: false,
+ schema: Arc::clone(input.schema()),
+ }),
+ )),
+ FilterPredicate::Expression(predicate) => Ok(Transformed::new_transformed(
+ LogicalPlan::Filter(Filter::new(predicate, input)),
+ simplified.transformed,
+ )),
}
}
}
+/// The outcome of a predicate when only rows for which it evaluates to `TRUE`
+/// are retained.
+enum FilterPredicate {
+ AcceptsAll,
+ RejectsAll(Expr),
+ Expression(Expr),
+}
+
+struct SimplifiedFilterPredicate {
+ predicate: FilterPredicate,
+ transformed: bool,
+}
+
+/// Simplifies positive `AND` / `OR` trees according to filter semantics.
+///
+/// A `NULL` predicate rejects a row just like `FALSE`. This equivalence is safe
+/// 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.
+fn simplify_filter_predicate(expr: Expr) -> SimplifiedFilterPredicate {
+ match expr {
+ Expr::Literal(ScalarValue::Boolean(Some(true)), _) => {
+ simplified(FilterPredicate::AcceptsAll, false)
+ }
+ expr @ Expr::Literal(ScalarValue::Boolean(Some(false) | None), _) => {
+ simplified(FilterPredicate::RejectsAll(expr), false)
+ }
+ Expr::BinaryExpr(binary) if binary.op == Operator::And => {
+ simplify_filter_and(*binary.left, *binary.right)
+ }
+ Expr::BinaryExpr(binary) if binary.op == Operator::Or => {
+ simplify_filter_or(*binary.left, *binary.right)
+ }
+ expr => simplified(FilterPredicate::Expression(expr), false),
+ }
+}
+
+fn simplify_filter_and(left: Expr, right: Expr) -> SimplifiedFilterPredicate {
+ let left = simplify_filter_predicate(left);
+ let right = simplify_filter_predicate(right);
+ let children_transformed = left.transformed || right.transformed;
+
+ match (left.predicate, right.predicate) {
+ (FilterPredicate::RejectsAll(left), FilterPredicate::RejectsAll(_)) => {
+ simplified(FilterPredicate::RejectsAll(left), true)
+ }
+ (FilterPredicate::RejectsAll(rejects), FilterPredicate::AcceptsAll)
+ | (FilterPredicate::AcceptsAll, FilterPredicate::RejectsAll(rejects)) => {
+ simplified(FilterPredicate::RejectsAll(rejects), true)
+ }
+ (FilterPredicate::RejectsAll(rejects), FilterPredicate::Expression(expr)) => {
+ if is_safe_to_discard(&expr) {
+ simplified(FilterPredicate::RejectsAll(rejects), true)
+ } else {
+ simplified(
+ FilterPredicate::Expression(rejects.and(expr)),
+ children_transformed,
+ )
+ }
+ }
+ (FilterPredicate::Expression(expr), FilterPredicate::RejectsAll(rejects)) => {
+ if is_safe_to_discard(&expr) {
+ simplified(FilterPredicate::RejectsAll(rejects), true)
+ } else {
+ simplified(
+ FilterPredicate::Expression(expr.and(rejects)),
+ children_transformed,
+ )
+ }
+ }
+ (FilterPredicate::AcceptsAll, predicate)
+ | (predicate, FilterPredicate::AcceptsAll) => simplified(predicate, true),
+ (FilterPredicate::Expression(left), FilterPredicate::Expression(right)) => {
+ simplified(
+ FilterPredicate::Expression(left.and(right)),
+ children_transformed,
+ )
+ }
+ }
+}
+
+fn simplify_filter_or(left: Expr, right: Expr) -> SimplifiedFilterPredicate {
+ let left = simplify_filter_predicate(left);
+ let right = simplify_filter_predicate(right);
+ let children_transformed = left.transformed || right.transformed;
+
+ match (left.predicate, right.predicate) {
+ (FilterPredicate::AcceptsAll, _) | (_, FilterPredicate::AcceptsAll) => {
+ simplified(FilterPredicate::AcceptsAll, true)
+ }
+ (FilterPredicate::RejectsAll(_), predicate)
+ | (predicate, FilterPredicate::RejectsAll(_)) => simplified(predicate, true),
+ (FilterPredicate::Expression(left), FilterPredicate::Expression(right)) => {
+ simplified(
+ FilterPredicate::Expression(left.or(right)),
+ children_transformed,
+ )
+ }
+ }
+}
+
+/// Returns true when skipping evaluation of `expr` cannot hide a runtime error
+/// or an observable side effect.
+///
+/// Logical expressions do not currently expose general infallibility metadata,
+/// so this is deliberately a small allowlist. In particular, arithmetic,
+/// casts, and function calls are retained even when another branch proves that
+/// a filter cannot accept a row.
+fn is_safe_to_discard(expr: &Expr) -> bool {
+ match expr {
+ Expr::Column(_) | Expr::Literal(_, _) => true,
+ Expr::BinaryExpr(binary)
+ if matches!(
+ binary.op,
+ Operator::Eq
+ | Operator::NotEq
+ | Operator::Lt
+ | Operator::LtEq
+ | Operator::Gt
+ | Operator::GtEq
+ | Operator::And
+ | Operator::Or
+ | Operator::IsDistinctFrom
+ | Operator::IsNotDistinctFrom
+ ) =>
+ {
+ is_safe_to_discard(&binary.left) && is_safe_to_discard(&binary.right)
+ }
+ Expr::Not(expr)
+ | Expr::IsNotNull(expr)
+ | Expr::IsNull(expr)
+ | Expr::IsTrue(expr)
+ | Expr::IsFalse(expr)
+ | Expr::IsUnknown(expr)
+ | Expr::IsNotTrue(expr)
+ | Expr::IsNotFalse(expr)
+ | Expr::IsNotUnknown(expr) => is_safe_to_discard(expr),
+ _ => false,
+ }
+}
+
+fn simplified(
+ predicate: FilterPredicate,
+ transformed: bool,
+) -> SimplifiedFilterPredicate {
+ SimplifiedFilterPredicate {
+ predicate,
+ transformed,
+ }
+}
+
#[cfg(test)]
mod tests {
+ use std::ops::Not;
use std::sync::Arc;
use crate::OptimizerContext;
@@ -86,7 +245,9 @@ mod tests {
use datafusion_common::{Result, ScalarValue};
use datafusion_expr::{Expr, col, lit, logical_plan::builder::LogicalPlanBuilder};
- use crate::eliminate_filter::EliminateFilter;
+ use crate::eliminate_filter::{
+ EliminateFilter, FilterPredicate, simplify_filter_predicate,
+ };
use crate::test::*;
use datafusion_expr::test::function_stub::sum;
@@ -134,6 +295,124 @@ mod tests {
assert_optimized_plan_equal!(plan, @"EmptyRelation: rows=0")
}
+ #[test]
+ fn filter_and_null() -> Result<()> {
+ let null = Expr::Literal(ScalarValue::Boolean(None), None);
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(col("b").and(null.clone()))?
+ .build()?;
+ assert_optimized_plan_equal!(plan, @"EmptyRelation: rows=0")?;
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(null.and(col("b")))?
+ .build()?;
+ assert_optimized_plan_equal!(plan, @"EmptyRelation: rows=0")?;
+
+ Ok(())
+ }
+
+ #[test]
+ fn filter_and_null_preserves_fallible_expression() -> Result<()> {
+ let null = Expr::Literal(ScalarValue::Boolean(None), None);
+ let fallible = (col("a") / lit(0u32)).gt(lit(0u32));
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(fallible.clone().and(null.clone()))?
+ .build()?;
+ assert_optimized_plan_equal!(
+ plan,
+ @r"
+ Filter: test.a / UInt32(0) > UInt32(0) AND Boolean(NULL)
+ TableScan: test
+ "
+ )?;
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(null.and(fallible))?
+ .build()?;
+ assert_optimized_plan_equal!(
+ plan,
+ @r"
+ Filter: Boolean(NULL) AND test.a / UInt32(0) > UInt32(0)
+ TableScan: test
+ "
+ )?;
+
+ Ok(())
+ }
+
+ #[test]
+ fn filter_or_null() -> Result<()> {
+ let null = Expr::Literal(ScalarValue::Boolean(None), None);
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(col("b").or(null.clone()))?
+ .build()?;
+ assert_optimized_plan_equal!(
+ plan,
+ @r"
+ Filter: test.b
+ TableScan: test
+ "
+ )?;
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(null.or(col("b")))?
+ .build()?;
+ assert_optimized_plan_equal!(
+ plan,
+ @r"
+ Filter: test.b
+ TableScan: test
+ "
+ )?;
+
+ Ok(())
+ }
+
+ #[test]
+ fn filter_nested_null_predicates() -> Result<()> {
+ let null = Expr::Literal(ScalarValue::Boolean(None), None);
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(col("b").and(col("b").or(null.clone())))?
+ .build()?;
+ assert_optimized_plan_equal!(
+ plan,
+ @r"
+ Filter: test.b AND test.b
+ TableScan: test
+ "
+ )?;
+
+ let plan = LogicalPlanBuilder::from(test_table_scan()?)
+ .filter(col("b").or(col("c").and(null)))?
+ .build()?;
+ assert_optimized_plan_equal!(
+ plan,
+ @r"
+ Filter: test.b
+ TableScan: test
+ "
+ )?;
+
+ Ok(())
+ }
+
+ #[test]
+ fn filter_null_simplification_does_not_cross_not() {
+ let null = Expr::Literal(ScalarValue::Boolean(None), None);
+ let predicate = col("b").or(null).not();
+ let result = simplify_filter_predicate(predicate.clone());
+
+ assert!(!result.transformed);
+ let FilterPredicate::Expression(actual) = result.predicate else {
+ panic!("expected the predicate to remain an expression")
+ };
+ assert_eq!(actual, predicate);
+ }
+
#[test]
fn filter_false_nested() -> Result<()> {
let filter_expr = lit(false);
diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
index a51a0c5b7d..5436bd0921 100644
--- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
+++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
@@ -1898,7 +1898,14 @@ impl TreeNodeRewriter for Simplifier<'_> {
{
match (*left, *right) {
(Expr::InList(l1), Expr::InList(l2)) => {
- return inlist_intersection(l1, &l2, false).map(Transformed::yes);
+ let simplified = inlist_intersection(l1.clone(), &l2, false)?;
+ if let Some(simplified) =
+ simplify_inlist_set_operation(info, &l1, &l2, simplified)?
+ {
+ Transformed::yes(simplified)
+ } else {
+ Transformed::no(Expr::InList(l1).and(Expr::InList(l2)))
+ }
}
// Matched previously once
_ => unreachable!(),
@@ -1938,7 +1945,14 @@ impl TreeNodeRewriter for Simplifier<'_> {
{
match (*left, *right) {
(Expr::InList(l1), Expr::InList(l2)) => {
- return inlist_except(l1, &l2).map(Transformed::yes);
+ let simplified = inlist_except(l1.clone(), &l2)?;
+ if let Some(simplified) =
+ simplify_inlist_set_operation(info, &l1, &l2, simplified)?
+ {
+ Transformed::yes(simplified)
+ } else {
+ Transformed::no(Expr::InList(l1).and(Expr::InList(l2)))
+ }
}
// Matched previously once
_ => unreachable!(),
@@ -1958,7 +1972,14 @@ impl TreeNodeRewriter for Simplifier<'_> {
{
match (*left, *right) {
(Expr::InList(l1), Expr::InList(l2)) => {
- return inlist_except(l2, &l1).map(Transformed::yes);
+ let simplified = inlist_except(l2.clone(), &l1)?;
+ if let Some(simplified) =
+ simplify_inlist_set_operation(info, &l1, &l2, simplified)?
+ {
+ Transformed::yes(simplified)
+ } else {
+ Transformed::no(Expr::InList(l1).and(Expr::InList(l2)))
+ }
}
// Matched previously once
_ => unreachable!(),
@@ -1978,7 +1999,14 @@ impl TreeNodeRewriter for Simplifier<'_> {
{
match (*left, *right) {
(Expr::InList(l1), Expr::InList(l2)) => {
- return inlist_intersection(l1, &l2, true).map(Transformed::yes);
+ let simplified = inlist_intersection(l1.clone(), &l2, true)?;
+ if let Some(simplified) =
+ simplify_inlist_set_operation(info, &l1, &l2, simplified)?
+ {
+ Transformed::yes(simplified)
+ } else {
+ Transformed::no(Expr::InList(l1).or(Expr::InList(l2)))
+ }
}
// Matched previously once
_ => unreachable!(),
@@ -2376,6 +2404,34 @@ fn inlist_except(mut l1: InList, l2: &InList) -> Result<Expr> {
Ok(Expr::InList(l1))
}
+/// Set algebra on `IN` lists is only sound if nullable list items are not
+/// discarded. When the tested expression is nullable, preserve the NULL branch
+/// of an otherwise constant result explicitly.
+fn simplify_inlist_set_operation(
+ info: &SimplifyContext,
+ left: &InList,
+ right: &InList,
+ result: Expr,
+) -> Result<Option<Expr>> {
+ for item in left.list.iter().chain(&right.list) {
+ if info.nullable(item)? {
+ return Ok(None);
+ }
+ }
+
+ if !is_true(&result) && !is_false(&result) {
+ return Ok(Some(result));
+ }
+
+ Ok(Some(if !info.nullable(left.expr.as_ref())? {
+ result
+ } else if is_true(&result) {
+ Expr::IsNotNull(left.expr.clone()).or(lit_bool_null())
+ } else {
+ Expr::IsNull(left.expr.clone()).and(lit_bool_null())
+ }))
+}
+
/// Returns expression testing a boolean `expr` for being exactly `true` (not `false` or NULL).
fn is_exactly_true(expr: Expr, info: &SimplifyContext) -> Result<Expr> {
if !info.nullable(&expr)? {
@@ -4676,11 +4732,24 @@ mod tests {
col("c1").eq(subquery1).or(col("c1").eq(subquery2))
);
- // 1. c1 IN (1,2,3,4) AND c1 IN (5,6,7,8) -> false
+ // 1. c1_non_null IN (1,2,3,4) AND c1_non_null IN (5,6,7,8) -> false
+ let expr = in_list(
+ col("c1_non_null"),
+ vec![lit(1), lit(2), lit(3), lit(4)],
+ false,
+ )
+ .and(in_list(
+ col("c1_non_null"),
+ vec![lit(5), lit(6), lit(7), lit(8)],
+ false,
+ ));
+ assert_eq!(simplify(expr), lit(false));
+
+ // Preserve the NULL branch when the tested expression is nullable.
let expr = in_list(col("c1"), vec![lit(1), lit(2), lit(3), lit(4)], false).and(
in_list(col("c1"), vec![lit(5), lit(6), lit(7), lit(8)], false),
);
- assert_eq!(simplify(expr), lit(false));
+ assert_eq!(simplify(expr), col("c1").is_null().and(lit_bool_null()));
// 2. c1 IN (1,2,3,4) AND c1 IN (4,5,6,7) -> c1 = 4
let expr = in_list(col("c1"), vec![lit(1), lit(2), lit(3), lit(4)], false).and(
@@ -4688,11 +4757,24 @@ mod tests {
);
assert_eq!(simplify(expr), col("c1").eq(lit(4)));
- // 3. c1 NOT IN (1, 2, 3, 4) OR c1 NOT IN (5, 6, 7, 8) -> true
+ // 3. c1_non_null NOT IN (1, 2, 3, 4) OR c1_non_null NOT IN (5, 6, 7, 8) -> true
+ let expr = in_list(
+ col("c1_non_null"),
+ vec![lit(1), lit(2), lit(3), lit(4)],
+ true,
+ )
+ .or(in_list(
+ col("c1_non_null"),
+ vec![lit(5), lit(6), lit(7), lit(8)],
+ true,
+ ));
+ assert_eq!(simplify(expr), lit(true));
+
+ // Preserve the NULL branch when the tested expression is nullable.
let expr = in_list(col("c1"), vec![lit(1), lit(2), lit(3), lit(4)], true).or(
in_list(col("c1"), vec![lit(5), lit(6), lit(7), lit(8)], true),
);
- assert_eq!(simplify(expr), lit(true));
+ assert_eq!(simplify(expr), col("c1").is_not_null().or(lit_bool_null()));
// 3.5 c1 NOT IN (1, 2, 3, 4) OR c1 NOT IN (4, 5, 6, 7) -> c1 != 4 (4 overlaps)
let expr = in_list(col("c1"), vec![lit(1), lit(2), lit(3), lit(4)], true).or(
@@ -4726,13 +4808,23 @@ mod tests {
)
);
- // 6. c1 IN (1,2,3) AND c1 NOT INT (1,2,3,4,5) -> false
+ // 6. c1_non_null IN (1,2,3) AND c1_non_null NOT IN (1,2,3,4,5) -> false
+ let expr = in_list(col("c1_non_null"), vec![lit(1), lit(2), lit(3)], false).and(
+ in_list(
+ col("c1_non_null"),
+ vec![lit(1), lit(2), lit(3), lit(4), lit(5)],
+ true,
+ ),
+ );
+ assert_eq!(simplify(expr), lit(false));
+
+ // Preserve the NULL branch when the tested expression is nullable.
let expr = in_list(col("c1"), vec![lit(1), lit(2), lit(3)], false).and(in_list(
col("c1"),
vec![lit(1), lit(2), lit(3), lit(4), lit(5)],
true,
));
- assert_eq!(simplify(expr), lit(false));
+ assert_eq!(simplify(expr), col("c1").is_null().and(lit_bool_null()));
// 7. c1 NOT IN (1,2,3,4) AND c1 IN (1,2,3,4,5) -> c1 = 5
let expr =
@@ -4798,6 +4890,64 @@ mod tests {
assert_eq!(simplify(expr.clone()), expr);
}
+ #[test]
+ fn simplify_inlist_set_operation_propagates_nullability_errors() {
+ let info = SimplifyContext::builder()
+ .with_schema(expr_test_schema())
+ .build();
+ let valid = InList {
+ expr: Box::new(col("c1")),
+ list: vec![lit("a")],
+ negated: false,
+ };
+
+ let invalid_list_item = InList {
+ expr: Box::new(col("c1")),
+ list: vec![col("missing")],
+ negated: false,
+ };
+ let error =
+ simplify_inlist_set_operation(&info, &invalid_list_item, &valid, lit(false))
+ .unwrap_err();
+ assert_contains!(error.to_string(), "No field named missing");
+
+ let invalid_tested_expr = InList {
+ expr: Box::new(col("missing")),
+ list: vec![lit("a")],
+ negated: false,
+ };
+ let error = simplify_inlist_set_operation(
+ &info,
+ &invalid_tested_expr,
+ &valid,
+ lit(false),
+ )
+ .unwrap_err();
+ assert_contains!(error.to_string(), "No field named missing");
+ }
+
+ #[test]
+ fn simplify_inlist_set_operation_preserves_null_result() {
+ let info = SimplifyContext::builder()
+ .with_schema(expr_test_schema())
+ .build();
+ let left = InList {
+ expr: Box::new(col("c1")),
+ list: vec![lit("a")],
+ negated: false,
+ };
+ let right = InList {
+ expr: Box::new(col("c1")),
+ list: vec![lit("b")],
+ negated: false,
+ };
+
+ assert_eq!(
+ simplify_inlist_set_operation(&info, &left, &right, lit_bool_null()).unwrap(),
+ Some(lit_bool_null())
+ );
+ }
+
#[test]
fn simplify_null_in_empty_inlist() {
// `NULL::boolean IN ()` == `NULL::boolean IN (SELECT foo FROM empty)` == false
diff --git a/datafusion/sqllogictest/test_files/array/array_has.slt b/datafusion/sqllogictest/test_files/array/array_has.slt
index 5bf6e5aaf1..d7b6680fab 100644
--- a/datafusion/sqllogictest/test_files/array/array_has.slt
+++ b/datafusion/sqllogictest/test_files/array/array_has.slt
@@ -634,8 +634,7 @@ select count(*) from test WHERE array_has([needle], needle);
----
100000
-# The optimizer does not currently eliminate the filter;
-# Instead, it's rewritten as `IS NULL OR NOT NULL` due to SQL null semantics
+# The filter retains the necessary null check for a nullable needle.
query TT
explain with test AS (SELECT substr(md5(i::text)::text, 1, 32) as needle FROM generate_series(1, 100000) t(i))
select count(*) from test WHERE array_has([needle], needle);
@@ -646,14 +645,14 @@ logical_plan
03)----SubqueryAlias: test
04)------SubqueryAlias: t
05)--------Projection:
-06)----------Filter: substr(CAST(md5(CAST(generate_series().value AS Utf8View)) AS Utf8View), Int64(1), Int64(32)) IS NOT NULL OR Boolean(NULL)
+06)----------Filter: substr(CAST(md5(CAST(generate_series().value AS Utf8View)) AS Utf8View), Int64(1), Int64(32)) IS NOT NULL
07)------------TableScan: generate_series() projection=[value]
physical_plan
01)ProjectionExec: expr=[count(Int64(1))@0 as count(*)]
02)--AggregateExec: mode=Final, gby=[], aggr=[count(Int64(1))]
03)----CoalescePartitionsExec
04)------AggregateExec: mode=Partial, gby=[], aggr=[count(Int64(1))]
-05)--------FilterExec: substr(md5(CAST(value@0 AS Utf8View)), 1, 32) IS NOT NULL OR NULL, projection=[]
+05)--------FilterExec: substr(md5(CAST(value@0 AS Utf8View)), 1, 32) IS NOT NULL, projection=[]
06)----------RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1
07)------------LazyMemoryExec: partitions=1, batch_generators=[generate_series: start=1, end=100000, batch_size=8192]
diff --git a/datafusion/sqllogictest/test_files/eliminate_outer_join.slt b/datafusion/sqllogictest/test_files/eliminate_outer_join.slt
index afd491b0b6..5d6bc68d1c 100644
--- a/datafusion/sqllogictest/test_files/eliminate_outer_join.slt
+++ b/datafusion/sqllogictest/test_files/eliminate_outer_join.slt
@@ -97,9 +97,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 = Int32(1) OR t1.a = Int32(2) OR Boolean(NULL)
+02)--Filter: t1.a = Int32(1) OR t1.a = Int32(2)
03)----TableScan: t1 projection=[a, b, c]
-04)--Filter: t2.x = Int32(1) OR t2.x = Int32(2) OR Boolean(NULL)
+04)--Filter: t2.x = Int32(1) OR t2.x = Int32(2)
05)----TableScan: t2 projection=[x, y, z]
query IITIIT rowsort
diff --git a/datafusion/sqllogictest/test_files/predicates.slt b/datafusion/sqllogictest/test_files/predicates.slt
index 9e28d1fb08..26105f3426 100644
--- a/datafusion/sqllogictest/test_files/predicates.slt
+++ b/datafusion/sqllogictest/test_files/predicates.slt
@@ -948,23 +948,26 @@ physical_plan EmptyExec
query TT
EXPLAIN FORMAT INDENT SELECT * FROM t WHERE x < 5 AND (10 * NULL < x);
----
-logical_plan
-01)Filter: t.x < Int32(5) AND Boolean(NULL)
-02)--TableScan: t projection=[x]
-physical_plan
-01)FilterExec: x@0 < 5 AND NULL
-02)--DataSourceExec: partitions=1, partition_sizes=[1]
+logical_plan EmptyRelation: rows=0
+physical_plan EmptyExec
query TT
EXPLAIN FORMAT INDENT SELECT * FROM t WHERE x < 5 OR (10 * NULL < x);
----
logical_plan
-01)Filter: t.x < Int32(5) OR Boolean(NULL)
+01)Filter: t.x < Int32(5)
02)--TableScan: t projection=[x]
physical_plan
-01)FilterExec: x@0 < 5 OR NULL
+01)FilterExec: x@0 < 5
02)--DataSourceExec: partitions=1, partition_sizes=[1]
+# A NULL branch rejects every row, but must not hide errors from the other branch.
+query error DataFusion error: Arrow error: Divide by zero error
+SELECT * FROM t WHERE x / 0 > 1 AND (10 * NULL < x);
+
+query error DataFusion error: Arrow error: Divide by zero error
+SELECT * FROM t WHERE (10 * NULL < x) AND x / 0 > 1;
+
statement ok
drop table t;
@@ -982,8 +985,12 @@ physical_plan EmptyExec
query TT
explain select x from t where x NOT IN (1,2,3,4) OR x NOT IN (5,6,7,8);
----
-logical_plan TableScan: t projection=[x]
-physical_plan DataSourceExec: partitions=1, partition_sizes=[1]
+logical_plan
+01)Filter: t.x IS NOT NULL
+02)--TableScan: t projection=[x]
+physical_plan
+01)FilterExec: x@0 IS NOT NULL
+02)--DataSourceExec: partitions=1, partition_sizes=[1]
query TT
explain select x from t where x IN (1,2,3,4,5) AND x NOT IN (1,2,3,4);
diff --git a/datafusion/sqllogictest/test_files/simplify_expr.slt b/datafusion/sqllogictest/test_files/simplify_expr.slt
index b24a1a6cff..c70ff2b955 100644
--- a/datafusion/sqllogictest/test_files/simplify_expr.slt
+++ b/datafusion/sqllogictest/test_files/simplify_expr.slt
@@ -34,21 +34,17 @@ query TT
explain select b from t where b ~ '.*'
----
logical_plan
-01)Filter: t.b IS NOT NULL OR Boolean(NULL)
+01)Filter: t.b IS NOT NULL
02)--TableScan: t projection=[b]
physical_plan
-01)FilterExec: b@0 IS NOT NULL OR NULL
+01)FilterExec: b@0 IS NOT NULL
02)--DataSourceExec: partitions=1, partition_sizes=[1]
query TT
explain select b from t where b !~ '.*'
----
-logical_plan
-01)Filter: t.b IS NULL AND Boolean(NULL)
-02)--TableScan: t projection=[b]
-physical_plan
-01)FilterExec: b@0 IS NULL AND NULL
-02)--DataSourceExec: partitions=1, partition_sizes=[1]
+logical_plan EmptyRelation: rows=0
+physical_plan EmptyExec
# `col ~ '.*'` is TRUE for any non-NULL string and NULL for a NULL input.
# The `.*` -> IS NOT NULL simplification must preserve that NULL (returning
@@ -97,10 +93,10 @@ query TT
explain select * from t where a = a;
----
logical_plan
-01)Filter: t.a IS NOT NULL OR Boolean(NULL)
+01)Filter: t.a IS NOT NULL
02)--TableScan: t projection=[a, b]
physical_plan
-01)FilterExec: a@0 IS NOT NULL OR NULL
+01)FilterExec: a@0 IS NOT NULL
02)--DataSourceExec: partitions=1, partition_sizes=[1]
statement ok
@@ -503,3 +499,22 @@ select id from date_unwrap where cast(d64 as date) = DATE '1970-01-01' order by
statement ok
drop table date_unwrap;
+
+# IN-list and array_has simplifications must preserve SQL NULL semantics.
+query BBB
+SELECT
+ i IN (1) AND i IN (2),
+ i NOT IN (1) OR i NOT IN (2),
+ array_has([i, 1], 2)
+FROM (VALUES (NULL::INT)) AS t(i);
+----
+NULL NULL false
+
+# Nullable IN-list items also prevent set-algebra rewrites.
+query BB
+SELECT
+ i IN (n, 1) AND i IN (2),
+ i NOT IN (n, 1) OR i NOT IN (2)
+FROM (VALUES (2, NULL::INT)) AS t(i, n);
+----
+NULL NULL
diff --git a/datafusion/sqllogictest/test_files/subquery.slt b/datafusion/sqllogictest/test_files/subquery.slt
index 9040d55f25..2da7483658 100644
--- a/datafusion/sqllogictest/test_files/subquery.slt
+++ b/datafusion/sqllogictest/test_files/subquery.slt
@@ -1781,11 +1781,11 @@ explain select v from (values (1), (6), (10)) set_cmp_t(v) where v > any(select
----
logical_plan
01)Projection: set_cmp_t.v
-02)--Filter: __correlated_sq_1.mark OR __correlated_sq_2.mark AND NOT __correlated_sq_3.mark AND Boolean(NULL)
-03)----LeftMark Join: Filter: set_cmp_t.v > __correlated_sq_3.v IS TRUE
-04)------Filter: __correlated_sq_1.mark OR __correlated_sq_2.mark AND Boolean(NULL)
-05)--------LeftMark Join: Filter: set_cmp_t.v > __correlated_sq_2.v IS NULL
-06)----------Filter: __correlated_sq_1.mark OR Boolean(NULL)
+02)--LeftMark Join: Filter: set_cmp_t.v > __correlated_sq_3.v IS TRUE
+03)----Projection: set_cmp_t.v
+04)------LeftMark Join: Filter: set_cmp_t.v > __correlated_sq_2.v IS NULL
+05)--------Projection: set_cmp_t.v
+06)----------Filter: __correlated_sq_1.mark
07)------------LeftMark Join: Filter: set_cmp_t.v > __correlated_sq_1.v IS TRUE
08)--------------SubqueryAlias: set_cmp_t
09)----------------Projection: column1 AS v
@@ -1794,14 +1794,14 @@ logical_plan
12)----------------SubqueryAlias: set_cmp_s
13)------------------Projection: column1 AS v
14)--------------------Values: (Int64(5)), (Int64(NULL))
-15)----------SubqueryAlias: __correlated_sq_2
-16)------------SubqueryAlias: set_cmp_s
-17)--------------Projection: column1 AS v
-18)----------------Values: (Int64(5)), (Int64(NULL))
-19)------SubqueryAlias: __correlated_sq_3
-20)--------SubqueryAlias: set_cmp_s
-21)----------Projection: column1 AS v
-22)------------Values: (Int64(5)), (Int64(NULL))
+15)--------SubqueryAlias: __correlated_sq_2
+16)----------SubqueryAlias: set_cmp_s
+17)------------Projection: column1 AS v
+18)--------------Values: (Int64(5)), (Int64(NULL))
+19)----SubqueryAlias: __correlated_sq_3
+20)------SubqueryAlias: set_cmp_s
+21)--------Projection: column1 AS v
+22)----------Values: (Int64(5)), (Int64(NULL))
# same-table `= ANY` / `<> ALL` must plan without
# "duplicate unqualified field name mark".
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org