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-24516-858d3bf7f9d36b0668946db7fb9c006892a95309 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 4798476d5e63aadb0fa4070c8a335ebcec9e8a37 Author: Fredrik Fornwall <[email protected]> AuthorDate: Thu Aug 20 13:50:52 2026 +0000 fix: account for empty scalar subqueries in nullability (#24516) ## Which issue does this PR close? - Closes apache/datafusion#24513. ## Rationale for this change A scalar subquery that returns zero rows evaluates to `NULL`, even when its projected expression is non-nullable. DataFusion currently derives `Expr::ScalarSubquery` nullability from the subquery's output field. This can produce a non-nullable output schema containing `NULL`, and can cause `SimplifyExpressions` to incorrectly fold predicates such as: ```sql SELECT (SELECT 1 WHERE FALSE) IS NULL; ``` to `false`. This change deliberately marks all scalar subqueries nullable, including those guaranteed to return exactly one row (such as an ungrouped aggregate like `(SELECT count(*) FROM t)`). This is a conservative trade-off that gives up some nullability precision for correctness, and is consistent with how PostgreSQL treats scalar subqueries. A possible follow-up refinement is a `LogicalPlan::min_rows()` lower bound (mirroring the existing `max_rows()`), which would let uncorrelated scalar subqueries provably returning at least one row keep their projected field's nullability. ## What changes are included in this PR? Scalar subqueries are conservatively marked nullable in logical expression schema derivation and physical expression planning. The projected field's data type, name, and metadata are preserved. ## Are these changes tested? Yes. Unit tests cover logical schema derivation and expression simplification, and SQLLogicTests cover both zero-row execution and `IS NULL` correctness. The full workspace test suite and Clippy with warnings denied pass. ## Are there any user-facing changes? Yes. Zero-row scalar subqueries with non-nullable projections now return `NULL` without a schema validation error, and `IS NULL` predicates produce the correct result. In addition, output schemas containing scalar subqueries now always mark those fields as nullable, even for subqueries that can never produce `NULL`. Downstream consumers that inspect schema nullability can observe this change. No public APIs change. --- AI usage: Created with Claude Code and Opus 5. I have reviewed the code and made modifications where it made sense. --------- Signed-off-by: Fredrik Fornwall <[email protected]> --- datafusion/expr/src/expr_schema.rs | 37 ++++++++++++++++++---- .../src/simplify_expressions/expr_simplifier.rs | 15 +++++++++ datafusion/physical-expr/src/planner.rs | 4 +-- datafusion/sqllogictest/test_files/subquery.slt | 33 +++++++++++++++++++ 4 files changed, 80 insertions(+), 9 deletions(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index ee70e90153..8a9d0b8937 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -369,9 +369,9 @@ impl ExprSchemable for Expr { Ok(expr_nullable | subquery_nullable) } - Expr::ScalarSubquery(subquery) => { - Ok(subquery.subquery.schema().field(0).is_nullable()) - } + // A scalar subquery may return no rows, in which case it evaluates to NULL + // regardless of the nullability of its projected field. + Expr::ScalarSubquery(_) => Ok(true), Expr::BinaryExpr(BinaryExpr { left, right, .. }) => { Ok(left.nullable(input_schema)? || right.nullable(input_schema)?) } @@ -517,9 +517,15 @@ impl ExprSchemable for Expr { | Expr::Exists { .. } => { Ok(Arc::new(Field::new(&schema_name, DataType::Boolean, false))) } - Expr::ScalarSubquery(subquery) => { - Ok(Arc::clone(&subquery.subquery.schema().fields()[0])) - } + Expr::ScalarSubquery(subquery) => Ok(Arc::new( + subquery + .subquery + .schema() + .field(0) + .as_ref() + .clone() + .with_nullable(true), + )), Expr::BinaryExpr(BinaryExpr { left, right, op }) => { let (left_field, right_field) = (left.to_field(schema)?.1, right.to_field(schema)?.1); @@ -800,7 +806,7 @@ mod tests { use crate::logical_plan::builder::LogicalTableSource; use crate::{ LogicalPlanBuilder, and, col, in_subquery, lit, not, or, - out_ref_col_with_metadata, when, + out_ref_col_with_metadata, scalar_subquery, when, }; use arrow::datatypes::Schema; @@ -1268,6 +1274,23 @@ mod tests { ); } + #[test] + fn scalar_subquery_is_nullable_with_non_nullable_output() { + let subquery = LogicalPlanBuilder::empty(false) + .project(vec![lit(1)]) + .unwrap() + .build() + .unwrap(); + assert!(!subquery.schema().field(0).is_nullable()); + + let expr = scalar_subquery(Arc::new(subquery)); + assert!(expr.nullable(&MockExprSchema::new()).unwrap()); + + let field = expr.to_field(&MockExprSchema::new()).unwrap().1; + assert_eq!(field.data_type(), &DataType::Int32); + assert!(field.is_nullable()); + } + #[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 b7e9624946..5beb54b560 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -3788,6 +3788,21 @@ mod tests { ); } + #[test] + fn simplify_scalar_subquery_is_null() { + let subquery = LogicalPlanBuilder::empty(false) + .project(vec![lit(1)]) + .unwrap() + .build() + .unwrap(); + let scalar_subquery = scalar_subquery(Arc::new(subquery)); + + assert_eq!( + simplify(scalar_subquery.clone().is_null()), + scalar_subquery.is_null() + ); + } + #[test] fn simplify_expr_is_unknown() { assert_eq!(simplify(col("c2").is_unknown()), col("c2").is_unknown(),); diff --git a/datafusion/physical-expr/src/planner.rs b/datafusion/physical-expr/src/planner.rs index f80d1b15bd..09b2e5ce04 100644 --- a/datafusion/physical-expr/src/planner.rs +++ b/datafusion/physical-expr/src/planner.rs @@ -536,10 +536,10 @@ pub fn create_physical_expr( ); } let dt = schema.field(0).data_type().clone(); - let nullable = schema.field(0).is_nullable(); Ok(Arc::new(ScalarSubqueryExpr::new( dt, - nullable, + // A scalar subquery may return no rows and evaluate to NULL. + true, index, planning_ctx.results().clone(), ))) diff --git a/datafusion/sqllogictest/test_files/subquery.slt b/datafusion/sqllogictest/test_files/subquery.slt index c38fb4c7ee..65760886d1 100644 --- a/datafusion/sqllogictest/test_files/subquery.slt +++ b/datafusion/sqllogictest/test_files/subquery.slt @@ -2010,6 +2010,24 @@ SELECT (SELECT v FROM sq_empty); ---- NULL +# A zero-row scalar subquery is nullable even when its projected expression is not. +query I +SELECT (SELECT 1 WHERE FALSE); +---- +NULL + +# Its nullability must prevent SimplifyExpressions from folding IS NULL to false. +query B +SELECT (SELECT 1 WHERE FALSE) IS NULL; +---- +true + +# The same holds for the opposite fold direction. +query B +SELECT (SELECT 1 WHERE FALSE) IS NOT NULL; +---- +false + # Scalar subquery returning zero rows in arithmetic → NULL propagation query I SELECT x + (SELECT v FROM sq_empty) FROM sq_main; @@ -2350,6 +2368,21 @@ SELECT (SELECT v FROM sq_empty); ---- NULL +# A zero-row scalar subquery is nullable even when its projected expression is +# not. The rewrite to a left join already produces a nullable column here, so +# this pins both paths to the same result. +query I +SELECT (SELECT 1 WHERE FALSE); +---- +NULL + +# SimplifyExpressions runs before ScalarSubqueryToJoin, so this fold is still +# governed by Expr::ScalarSubquery nullability on this path. +query B +SELECT (SELECT 1 WHERE FALSE) IS NULL; +---- +true + # Scalar subquery returning zero rows in arithmetic → NULL propagation query I SELECT x + (SELECT v FROM sq_empty) FROM sq_main; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
