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/08/24 13:21:00 UTC
(datafusion) 01/01: Fix physical distinctness expression nullability (#24244)
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-24244-3dc98540f5d33926d6ab0f56b5e2b637532c10c6
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit fdf7935a640e557147d5a22268b237021c2d904b
Author: blinding-pixels <bl...@protonmail.com>
AuthorDate: Mon Aug 24 13:20:35 2026 +0000
Fix physical distinctness expression nullability (#24244)
## Which issue does this PR close?
- Closes #24096
## Rationale for this change
When aggregate CSE extracts a repeated truth check, the logical and
physical plans assign different nullability metadata to it, causing
planning to fail. Since these truth checks always return either true or
false, the physical plan should mark them as non-nullable.
## What changes are included in this PR?
This PR updates physical `BinaryExpr` nullability so `IS DISTINCT FROM`
and `IS NOT DISTINCT FROM` are always reported as non-nullable.
It also adds a focused unit-test matrix and a SQL logic regression
covering the aggregate CSE path reported in the issue.
## Are these changes tested?
Yes. The tests cover both distinctness operators, ordinary equality as a
control, and the reported aggregate query.
Formatting, Clippy, focused tests, and the extended workspace test suite
all pass.
## Are there any user-facing changes?
Yes. Affected queries using repeated truth checks on nullable Boolean
values now plan and execute instead of failing with a logical/physical
schema mismatch.
There are no public API changes.
---------
Co-authored-by: blinding-pixels <28...@users.noreply.github.com>
---
datafusion/expr/src/expr_schema.rs | 47 +++++++++++++++++-----
datafusion/physical-expr/src/expressions/binary.rs | 29 ++++++++++++-
datafusion/sqllogictest/test_files/scalar.slt | 20 +++++++++
3 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs
index 21c68089df..36b76f076d 100644
--- a/datafusion/expr/src/expr_schema.rs
+++ b/datafusion/expr/src/expr_schema.rs
@@ -27,7 +27,9 @@ use crate::higher_order_function::HigherOrderReturnFieldArgs;
use crate::type_coercion::functions::value_fields_with_higher_order_udf_and_lambdas;
use crate::type_coercion::functions::{UDFCoercionExt, fields_with_udf};
use crate::udf::ReturnFieldArgs;
-use crate::{LogicalPlan, Projection, Subquery, WindowFunctionDefinition, utils};
+use crate::{
+ LogicalPlan, Operator, Projection, Subquery, WindowFunctionDefinition, utils,
+};
use arrow::compute::can_cast_types;
use arrow::datatypes::FieldRef;
use arrow::datatypes::{DataType, Field};
@@ -390,9 +392,10 @@ impl ExprSchemable for Expr {
Ok(expr_nullable | subquery_nullable)
}
Expr::ScalarSubquery(subquery) => Ok(scalar_subquery_nullable(subquery)),
- Expr::BinaryExpr(BinaryExpr { left, right, .. }) => {
- Ok(left.nullable(input_schema)? || right.nullable(input_schema)?)
- }
+ Expr::BinaryExpr(BinaryExpr { left, right, op }) => match op {
+ Operator::IsDistinctFrom | Operator::IsNotDistinctFrom => Ok(false),
+ _ => Ok(left.nullable(input_schema)? || right.nullable(input_schema)?),
+ },
Expr::Like(Like { expr, pattern, .. })
| Expr::SimilarTo(Like { expr, pattern, .. }) => {
Ok(expr.nullable(input_schema)? || pattern.nullable(input_schema)?)
@@ -555,10 +558,14 @@ impl ExprSchemable for Expr {
let mut coercer = BinaryTypeCoercer::new(lhs_type, op, rhs_type);
coercer.set_lhs_spans(left.spans().cloned().unwrap_or_default());
coercer.set_rhs_spans(right.spans().cloned().unwrap_or_default());
+ let nullable = match op {
+ Operator::IsDistinctFrom | Operator::IsNotDistinctFrom => false,
+ _ => lhs_nullable || rhs_nullable,
+ };
Ok(Arc::new(Field::new(
&schema_name,
coercer.get_result_type()?,
- lhs_nullable || rhs_nullable,
+ nullable,
)))
}
Expr::WindowFunction(window_function) => {
@@ -899,12 +906,30 @@ mod tests {
#[test]
fn expr_schema_nullability() {
- let expr = col("foo").eq(lit(1));
- assert!(!expr.nullable(&MockExprSchema::new()).unwrap());
- assert!(
- expr.nullable(&MockExprSchema::new().with_nullable(true))
- .unwrap()
- );
+ let cases = [
+ (Operator::Eq, false, false),
+ (Operator::Eq, true, true),
+ (Operator::IsDistinctFrom, true, false),
+ (Operator::IsNotDistinctFrom, true, false),
+ ];
+
+ for (op, input_nullable, expected) in cases {
+ let expr = Expr::BinaryExpr(BinaryExpr::new(
+ Box::new(col("foo")),
+ op,
+ Box::new(col("bar")),
+ ));
+ let schema = MockExprSchema::new()
+ .with_data_type(DataType::Boolean)
+ .with_nullable(input_nullable);
+
+ assert_eq!(expr.nullable(&schema).unwrap(), expected, "{op}");
+ assert_eq!(
+ expr.to_field(&schema).unwrap().1.is_nullable(),
+ expected,
+ "{op}"
+ );
+ }
test_is_expr_nullable!(is_null);
test_is_expr_nullable!(is_not_null);
diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs
index c956b83178..dfb1d136d0 100644
--- a/datafusion/physical-expr/src/expressions/binary.rs
+++ b/datafusion/physical-expr/src/expressions/binary.rs
@@ -530,7 +530,13 @@ impl PhysicalExpr for BinaryExpr {
}
fn nullable(&self, input_schema: &Schema) -> Result<bool> {
- Ok(self.left.nullable(input_schema)? || self.right.nullable(input_schema)?)
+ match self.op {
+ Operator::IsDistinctFrom | Operator::IsNotDistinctFrom => Ok(false),
+ _ => {
+ Ok(self.left.nullable(input_schema)?
+ || self.right.nullable(input_schema)?)
+ }
+ }
}
fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
@@ -4193,6 +4199,27 @@ mod tests {
apply_logic_op(&schema, &a, &b, Operator::IsDistinctFrom, expected).unwrap();
}
+ #[test]
+ fn distinct_from_op_nullability() -> Result<()> {
+ let schema = Schema::new(vec![
+ Field::new("nullable", DataType::Boolean, true),
+ Field::new("non_nullable", DataType::Boolean, false),
+ ]);
+ let cases = [
+ (Operator::IsDistinctFrom, "nullable", false),
+ (Operator::IsNotDistinctFrom, "nullable", false),
+ (Operator::Eq, "nullable", true),
+ (Operator::Eq, "non_nullable", false),
+ ];
+
+ for (op, column, expected) in cases {
+ let expr = BinaryExpr::new(col(column, &schema)?, op, lit(true));
+ assert_eq!(expr.nullable(&schema)?, expected, "{op} with {column}");
+ }
+
+ Ok(())
+ }
+
#[test]
fn is_not_distinct_from_op_bool() {
let (schema, a, b) = bool_test_arrays();
diff --git a/datafusion/sqllogictest/test_files/scalar.slt b/datafusion/sqllogictest/test_files/scalar.slt
index 2bedb28a85..175607b620 100644
--- a/datafusion/sqllogictest/test_files/scalar.slt
+++ b/datafusion/sqllogictest/test_files/scalar.slt
@@ -1864,6 +1864,26 @@ true
false
true
+# Aggregate CSE preserves the non-nullability of truth predicates
+# issue: https://github.com/apache/datafusion/issues/24096
+query IIIIIII
+SELECT
+ SUM(CASE WHEN b IS TRUE THEN 1 ELSE 0 END),
+ COUNT(CASE WHEN b IS TRUE THEN 1 END),
+ SUM(CASE WHEN b IS FALSE THEN 1 ELSE 0 END),
+ SUM(CASE WHEN b IS NOT TRUE THEN 1 ELSE 0 END),
+ SUM(CASE WHEN b IS NOT FALSE THEN 1 ELSE 0 END),
+ SUM(CASE WHEN b IS UNKNOWN THEN 1 ELSE 0 END),
+ SUM(CASE WHEN b IS NOT UNKNOWN THEN 1 ELSE 0 END)
+FROM (
+ VALUES
+ (TRUE),
+ (FALSE),
+ (CAST(NULL AS BOOLEAN))
+) AS t(b);
+----
+1 1 1 2 2 1 2
+
# query_without_from()
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org