From 51f7dc022a39748ea30e7a862918231e0adb6fd2 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Thu, 9 Jul 2026 19:39:46 +0100 Subject: [PATCH 1/4] Tests Signed-off-by: Adam Gutglick --- datafusion/expr/src/expr_schema.rs | 65 ++++++++++++++++++- .../simplify_expressions/expr_simplifier.rs | 30 +++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index ec367de846d63..a3a1c025ad6ff 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -796,8 +796,13 @@ mod tests { use std::collections::HashMap; use super::*; - use crate::{and, col, lit, not, or, out_ref_col_with_metadata, when}; + use crate::logical_plan::builder::LogicalTableSource; + use crate::{ + LogicalPlanBuilder, and, col, in_subquery, lit, not, or, + out_ref_col_with_metadata, when, + }; + use arrow::datatypes::Schema; use datafusion_common::{DFSchema, assert_or_internal_err}; macro_rules! test_is_expr_nullable { @@ -1192,6 +1197,64 @@ mod tests { } } + /// A scan of `t`, whose single column `a` has the given nullability. + fn scan_t(a_nullable: bool) -> LogicalPlanBuilder { + let schema = Schema::new(vec![Field::new("a", DataType::Int32, a_nullable)]); + let source = Arc::new(LogicalTableSource::new(Arc::new(schema))); + LogicalPlanBuilder::scan("t", source, None).unwrap() + } + + #[test] + fn in_subquery_nullability() { + // `x IN (SELECT a FROM t)` evaluates to NULL when `x` is NULL, and when `x` + // matches no row while `a` contains a NULL. So it is nullable exactly when + // either the compared expression or the subquery's output column is. + let cases = [ + (false, false, false), + (false, true, true), + (true, false, true), + (true, true, true), + ]; + + for (x_nullable, a_nullable, expected) in cases { + let subquery = scan_t(a_nullable) + .project(vec![col("a")]) + .unwrap() + .build() + .unwrap(); + let expr = in_subquery(col("x"), Arc::new(subquery)); + let schema = MockExprSchema::new().with_nullable(x_nullable); + + assert_eq!(expr.nullable(&schema).unwrap(), expected); + } + } + + #[test] + fn in_subquery_nullability_uses_subquery_output_schema() { + // `DISTINCT` carries no expressions of its own, but its output column is still + // nullable, so the `IN` expression must be nullable too. + let subquery = scan_t(true) + .project(vec![col("a")]) + .unwrap() + .distinct() + .unwrap() + .build() + .unwrap(); + let expr = in_subquery(col("x"), Arc::new(subquery)); + assert!(expr.nullable(&MockExprSchema::new()).unwrap()); + + // A computed projection's expressions reference `t.a`, which does not appear in + // the subquery's output schema, so nullability must be read off that schema's + // single column rather than by resolving the projection's expressions against it. + let subquery = scan_t(false) + .project(vec![col("a") + lit(1)]) + .unwrap() + .build() + .unwrap(); + let expr = in_subquery(col("x"), Arc::new(subquery)); + assert!(!expr.nullable(&MockExprSchema::new()).unwrap()); + } + #[test] fn test_scalar_variable() { let mut meta = HashMap::new(); diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index f5ea75dde8612..2b606687d47a3 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -2546,6 +2546,36 @@ mod tests { assert_eq!(simplify(expr_b), expected_b); } + /// `c3_non_null IN (SELECT a FROM t)`, where `a` has the given nullability. + fn in_subquery_expr(a_nullable: bool) -> Expr { + let schema = Schema::new(vec![Field::new("a", DataType::Int64, a_nullable)]); + let source = Arc::new(LogicalTableSource::new(Arc::new(schema))); + let subquery = LogicalPlanBuilder::scan("t", source, None) + .unwrap() + .project(vec![col("a")]) + .unwrap() + .build() + .unwrap(); + + in_subquery(col("c3_non_null"), Arc::new(subquery)) + } + + #[test] + fn test_simplify_eq_not_self_in_subquery() { + // `expr_a`: even though `c3_non_null` is non-nullable, the `IN` evaluates to NULL + // when `c3_non_null` matches no row and the subquery's `a` contains a NULL. So the + // expression is nullable and `A = A` must not fold to `true`. + let expr_a = in_subquery_expr(true); + let expected_a = expr_a.clone().is_not_null().or(lit_bool_null()); + + // `expr_b`: neither side can be NULL, so the `IN` is non-nullable and `A = A` is true. + let expr_b = in_subquery_expr(false); + let expected_b = lit(true); + + assert_eq!(simplify(expr_a.clone().eq(expr_a)), expected_a); + assert_eq!(simplify(expr_b.clone().eq(expr_b)), expected_b); + } + #[test] fn test_simplify_or_true() { let expr_a = col("c2").or(lit(true)); From b36ddb70225fd35f37944fb5b6c4f09c74732f08 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Thu, 9 Jul 2026 19:40:02 +0100 Subject: [PATCH 2/4] Fix Signed-off-by: Adam Gutglick --- datafusion/expr/src/expr_schema.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index a3a1c025ad6ff..a4ba6fff8414b 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -366,7 +366,12 @@ impl ExprSchemable for Expr { | Expr::IsNotUnknown(_) | Expr::Exists { .. } => Ok(false), Expr::SetComparison(_) => Ok(true), - Expr::InSubquery(InSubquery { expr, .. }) => expr.nullable(input_schema), + Expr::InSubquery(InSubquery { expr, subquery, .. }) => { + let expr_nullable = expr.nullable(input_schema)?; + let subquery_nullable = subquery.subquery.schema().field(0).is_nullable(); + + Ok(expr_nullable | subquery_nullable) + } Expr::ScalarSubquery(subquery) => { Ok(subquery.subquery.schema().field(0).is_nullable()) } From 9dcebded7a637d29aace3b418372fc293b99b363 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Wed, 5 Aug 2026 12:59:04 +0100 Subject: [PATCH 3/4] Use checked field access Signed-off-by: Adam Gutglick --- datafusion/expr/src/expr_schema.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index a4ba6fff8414b..4a6f092f61686 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -34,7 +34,7 @@ use arrow::datatypes::{DataType, Field}; use datafusion_common::datatype::FieldExt; use datafusion_common::{ Column, DataFusionError, ExprSchema, Result, ScalarValue, Spans, TableReference, - not_impl_err, plan_datafusion_err, plan_err, + exec_datafusion_err, not_impl_err, plan_datafusion_err, plan_err, }; use datafusion_expr_common::type_coercion::binary::BinaryTypeCoercer; use datafusion_functions_window_common::field::WindowUDFFieldArgs; @@ -368,7 +368,9 @@ impl ExprSchemable for Expr { Expr::SetComparison(_) => Ok(true), Expr::InSubquery(InSubquery { expr, subquery, .. }) => { let expr_nullable = expr.nullable(input_schema)?; - let subquery_nullable = subquery.subquery.schema().field(0).is_nullable(); + let subquery_nullable = subquery.subquery.schema().fields().first().ok_or_else(|| { + exec_datafusion_err!("subquery must return exactly one column of data to compare against") + })?.is_nullable(); Ok(expr_nullable | subquery_nullable) } From 37a5fbbf7c4d8ff0002014b29d8ebf29dde9fab7 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Wed, 5 Aug 2026 14:17:37 +0100 Subject: [PATCH 4/4] plan error + test Signed-off-by: Adam Gutglick --- datafusion/expr/src/expr_schema.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index 4a6f092f61686..8927fcf4d0bbe 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -34,7 +34,7 @@ use arrow::datatypes::{DataType, Field}; use datafusion_common::datatype::FieldExt; use datafusion_common::{ Column, DataFusionError, ExprSchema, Result, ScalarValue, Spans, TableReference, - exec_datafusion_err, not_impl_err, plan_datafusion_err, plan_err, + not_impl_err, plan_datafusion_err, plan_err, }; use datafusion_expr_common::type_coercion::binary::BinaryTypeCoercer; use datafusion_functions_window_common::field::WindowUDFFieldArgs; @@ -369,7 +369,7 @@ impl ExprSchemable for Expr { Expr::InSubquery(InSubquery { expr, subquery, .. }) => { let expr_nullable = expr.nullable(input_schema)?; let subquery_nullable = subquery.subquery.schema().fields().first().ok_or_else(|| { - exec_datafusion_err!("subquery must return exactly one column of data to compare against") + plan_datafusion_err!("subquery must return exactly one column of data to compare against") })?.is_nullable(); Ok(expr_nullable | subquery_nullable) @@ -1262,6 +1262,18 @@ mod tests { assert!(!expr.nullable(&MockExprSchema::new()).unwrap()); } + #[test] + fn in_subquery_nullability_errors_for_no_subquery_columns() { + let subquery = LogicalPlanBuilder::empty(false).build().unwrap(); + let expr = in_subquery(col("x"), Arc::new(subquery)); + + let err = expr.nullable(&MockExprSchema::new()).unwrap_err(); + assert_eq!( + err.strip_backtrace(), + "Error during planning: subquery must return exactly one column of data to compare against" + ); + } + #[test] fn test_scalar_variable() { let mut meta = HashMap::new();