Skip to content

Commit 4798476

Browse files
authored
fix: account for empty scalar subqueries in nullability (#24516)
## Which issue does this PR close? - Closes #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 <fredrik@fornwall.net>
1 parent 858d3bf commit 4798476

4 files changed

Lines changed: 80 additions & 9 deletions

File tree

datafusion/expr/src/expr_schema.rs

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,9 @@ impl ExprSchemable for Expr {
369369

370370
Ok(expr_nullable | subquery_nullable)
371371
}
372-
Expr::ScalarSubquery(subquery) => {
373-
Ok(subquery.subquery.schema().field(0).is_nullable())
374-
}
372+
// A scalar subquery may return no rows, in which case it evaluates to NULL
373+
// regardless of the nullability of its projected field.
374+
Expr::ScalarSubquery(_) => Ok(true),
375375
Expr::BinaryExpr(BinaryExpr { left, right, .. }) => {
376376
Ok(left.nullable(input_schema)? || right.nullable(input_schema)?)
377377
}
@@ -517,9 +517,15 @@ impl ExprSchemable for Expr {
517517
| Expr::Exists { .. } => {
518518
Ok(Arc::new(Field::new(&schema_name, DataType::Boolean, false)))
519519
}
520-
Expr::ScalarSubquery(subquery) => {
521-
Ok(Arc::clone(&subquery.subquery.schema().fields()[0]))
522-
}
520+
Expr::ScalarSubquery(subquery) => Ok(Arc::new(
521+
subquery
522+
.subquery
523+
.schema()
524+
.field(0)
525+
.as_ref()
526+
.clone()
527+
.with_nullable(true),
528+
)),
523529
Expr::BinaryExpr(BinaryExpr { left, right, op }) => {
524530
let (left_field, right_field) =
525531
(left.to_field(schema)?.1, right.to_field(schema)?.1);
@@ -800,7 +806,7 @@ mod tests {
800806
use crate::logical_plan::builder::LogicalTableSource;
801807
use crate::{
802808
LogicalPlanBuilder, and, col, in_subquery, lit, not, or,
803-
out_ref_col_with_metadata, when,
809+
out_ref_col_with_metadata, scalar_subquery, when,
804810
};
805811

806812
use arrow::datatypes::Schema;
@@ -1268,6 +1274,23 @@ mod tests {
12681274
);
12691275
}
12701276

1277+
#[test]
1278+
fn scalar_subquery_is_nullable_with_non_nullable_output() {
1279+
let subquery = LogicalPlanBuilder::empty(false)
1280+
.project(vec![lit(1)])
1281+
.unwrap()
1282+
.build()
1283+
.unwrap();
1284+
assert!(!subquery.schema().field(0).is_nullable());
1285+
1286+
let expr = scalar_subquery(Arc::new(subquery));
1287+
assert!(expr.nullable(&MockExprSchema::new()).unwrap());
1288+
1289+
let field = expr.to_field(&MockExprSchema::new()).unwrap().1;
1290+
assert_eq!(field.data_type(), &DataType::Int32);
1291+
assert!(field.is_nullable());
1292+
}
1293+
12711294
#[test]
12721295
fn test_scalar_variable() {
12731296
let mut meta = HashMap::new();

datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3788,6 +3788,21 @@ mod tests {
37883788
);
37893789
}
37903790

3791+
#[test]
3792+
fn simplify_scalar_subquery_is_null() {
3793+
let subquery = LogicalPlanBuilder::empty(false)
3794+
.project(vec![lit(1)])
3795+
.unwrap()
3796+
.build()
3797+
.unwrap();
3798+
let scalar_subquery = scalar_subquery(Arc::new(subquery));
3799+
3800+
assert_eq!(
3801+
simplify(scalar_subquery.clone().is_null()),
3802+
scalar_subquery.is_null()
3803+
);
3804+
}
3805+
37913806
#[test]
37923807
fn simplify_expr_is_unknown() {
37933808
assert_eq!(simplify(col("c2").is_unknown()), col("c2").is_unknown(),);

datafusion/physical-expr/src/planner.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,10 +536,10 @@ pub fn create_physical_expr(
536536
);
537537
}
538538
let dt = schema.field(0).data_type().clone();
539-
let nullable = schema.field(0).is_nullable();
540539
Ok(Arc::new(ScalarSubqueryExpr::new(
541540
dt,
542-
nullable,
541+
// A scalar subquery may return no rows and evaluate to NULL.
542+
true,
543543
index,
544544
planning_ctx.results().clone(),
545545
)))

datafusion/sqllogictest/test_files/subquery.slt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2010,6 +2010,24 @@ SELECT (SELECT v FROM sq_empty);
20102010
----
20112011
NULL
20122012

2013+
# A zero-row scalar subquery is nullable even when its projected expression is not.
2014+
query I
2015+
SELECT (SELECT 1 WHERE FALSE);
2016+
----
2017+
NULL
2018+
2019+
# Its nullability must prevent SimplifyExpressions from folding IS NULL to false.
2020+
query B
2021+
SELECT (SELECT 1 WHERE FALSE) IS NULL;
2022+
----
2023+
true
2024+
2025+
# The same holds for the opposite fold direction.
2026+
query B
2027+
SELECT (SELECT 1 WHERE FALSE) IS NOT NULL;
2028+
----
2029+
false
2030+
20132031
# Scalar subquery returning zero rows in arithmetic → NULL propagation
20142032
query I
20152033
SELECT x + (SELECT v FROM sq_empty) FROM sq_main;
@@ -2350,6 +2368,21 @@ SELECT (SELECT v FROM sq_empty);
23502368
----
23512369
NULL
23522370

2371+
# A zero-row scalar subquery is nullable even when its projected expression is
2372+
# not. The rewrite to a left join already produces a nullable column here, so
2373+
# this pins both paths to the same result.
2374+
query I
2375+
SELECT (SELECT 1 WHERE FALSE);
2376+
----
2377+
NULL
2378+
2379+
# SimplifyExpressions runs before ScalarSubqueryToJoin, so this fold is still
2380+
# governed by Expr::ScalarSubquery nullability on this path.
2381+
query B
2382+
SELECT (SELECT 1 WHERE FALSE) IS NULL;
2383+
----
2384+
true
2385+
23532386
# Scalar subquery returning zero rows in arithmetic → NULL propagation
23542387
query I
23552388
SELECT x + (SELECT v FROM sq_empty) FROM sq_main;

0 commit comments

Comments
 (0)