From 100e9b757c30a3a97fb6b09351bfb56bbf27d1c9 Mon Sep 17 00:00:00 2001 From: sriram Date: Tue, 25 Nov 2025 22:35:13 +0530 Subject: [PATCH 1/2] deprecate AggregateUDF::is_nullable --- datafusion/expr/src/expr_schema.rs | 5 +++-- datafusion/expr/src/udaf.rs | 11 ++++++++++- datafusion/physical-expr/src/aggregate.rs | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index 94f5b0480b65..725d7b0a3f8e 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -353,8 +353,9 @@ impl ExprSchemable for Expr { let (_, nullable) = self.data_type_and_nullable(input_schema)?; Ok(nullable) } - Expr::AggregateFunction(AggregateFunction { func, .. }) => { - Ok(func.is_nullable()) + Expr::AggregateFunction(_) => { + let (_, nullable) = self.data_type_and_nullable(input_schema)?; + Ok(nullable) } Expr::WindowFunction(window_function) => self .data_type_and_nullable_with_window_function( diff --git a/datafusion/expr/src/udaf.rs b/datafusion/expr/src/udaf.rs index e2ae697deedf..c756ee299832 100644 --- a/datafusion/expr/src/udaf.rs +++ b/datafusion/expr/src/udaf.rs @@ -209,6 +209,7 @@ impl AggregateUDF { } pub fn is_nullable(&self) -> bool { + #[allow(deprecated)] self.inner.is_nullable() } @@ -532,6 +533,10 @@ pub trait AggregateUDFImpl: Debug + DynEq + DynHash + Send + Sync { /// For example, aggregate functions like `COUNT` always return a non null value /// but others like `MIN` will return `NULL` if there is nullable input. /// Note that if the function is declared as *not* nullable, make sure the [`AggregateUDFImpl::default_value`] is `non-null` + #[deprecated( + since = "51.0.0", + note = "Use `return_field` instead with return_field.is_nullable()." + )] fn is_nullable(&self) -> bool { true } @@ -1100,7 +1105,10 @@ pub fn udaf_default_return_field( Ok(Arc::new(Field::new( func.name(), data_type, - func.is_nullable(), + #[allow(deprecated)] + { + func.is_nullable() + }, ))) } @@ -1247,6 +1255,7 @@ impl AggregateUDFImpl for AliasedAggregateUDFImpl { } fn is_nullable(&self) -> bool { + #[allow(deprecated)] self.inner.is_nullable() } diff --git a/datafusion/physical-expr/src/aggregate.rs b/datafusion/physical-expr/src/aggregate.rs index ae5a4a855947..aee329ab4b43 100644 --- a/datafusion/physical-expr/src/aggregate.rs +++ b/datafusion/physical-expr/src/aggregate.rs @@ -222,7 +222,7 @@ impl AggregateExprBuilder { )?; let return_field = fun.return_field(&input_exprs_fields)?; - let is_nullable = fun.is_nullable(); + let is_nullable = return_field.is_nullable(); let name = match alias { None => { return internal_err!( From 2aeedc8bb127db758bb26b8afa393d5eb39dd93a Mon Sep 17 00:00:00 2001 From: sriram Date: Wed, 26 Nov 2025 22:36:42 +0530 Subject: [PATCH 2/2] add missed out [#expect(deprecated)] assertions --- datafusion/expr/src/udaf.rs | 12 ++++++++---- datafusion/ffi/src/udaf/mod.rs | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/datafusion/expr/src/udaf.rs b/datafusion/expr/src/udaf.rs index c756ee299832..91437d60a250 100644 --- a/datafusion/expr/src/udaf.rs +++ b/datafusion/expr/src/udaf.rs @@ -208,8 +208,12 @@ impl AggregateUDF { self.inner.window_function_display_name(params) } + #[deprecated( + since = "52.0.0", + note = "Use `return_field` instead with return_field.is_nullable()." + )] pub fn is_nullable(&self) -> bool { - #[allow(deprecated)] + #[expect(deprecated)] self.inner.is_nullable() } @@ -534,7 +538,7 @@ pub trait AggregateUDFImpl: Debug + DynEq + DynHash + Send + Sync { /// but others like `MIN` will return `NULL` if there is nullable input. /// Note that if the function is declared as *not* nullable, make sure the [`AggregateUDFImpl::default_value`] is `non-null` #[deprecated( - since = "51.0.0", + since = "52.0.0", note = "Use `return_field` instead with return_field.is_nullable()." )] fn is_nullable(&self) -> bool { @@ -1105,7 +1109,7 @@ pub fn udaf_default_return_field( Ok(Arc::new(Field::new( func.name(), data_type, - #[allow(deprecated)] + #[expect(deprecated)] { func.is_nullable() }, @@ -1255,7 +1259,7 @@ impl AggregateUDFImpl for AliasedAggregateUDFImpl { } fn is_nullable(&self) -> bool { - #[allow(deprecated)] + #[expect(deprecated)] self.inner.is_nullable() } diff --git a/datafusion/ffi/src/udaf/mod.rs b/datafusion/ffi/src/udaf/mod.rs index a416753c371b..d254561fba34 100644 --- a/datafusion/ffi/src/udaf/mod.rs +++ b/datafusion/ffi/src/udaf/mod.rs @@ -344,6 +344,7 @@ impl From> for FFI_AggregateUDF { fn from(udaf: Arc) -> Self { let name = udaf.name().into(); let aliases = udaf.aliases().iter().map(|a| a.to_owned().into()).collect(); + #[expect(deprecated)] let is_nullable = udaf.is_nullable(); let volatility = udaf.signature().volatility.into();