Skip to content

Commit a03ca3f

Browse files
committed
Fill missing methods in aliased UDF impls
The aliased UDF impls -- `AliasedScalarUDFImpl`, `AliasedAggregateUDFImpl` and `AliasedWindowUDFImpl` are supposed to forward all methods (except `aliases` to the underlying implementation. However, they failed to do so, with many optional methods not being forwarded. This change fixes this for good. Adds the missing method implementations. Replaces a code comment with compile-time check that all methods are indeed implemented.
1 parent f0630fb commit a03ca3f

3 files changed

Lines changed: 67 additions & 10 deletions

File tree

datafusion/expr/src/udaf.rs

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -427,9 +427,6 @@ where
427427
/// let expr = geometric_mean.call(vec![col("a")]);
428428
/// ```
429429
pub trait AggregateUDFImpl: Debug + Send + Sync {
430-
// Note: When adding any methods (with default implementations), remember to add them also
431-
// into the AliasedAggregateUDFImpl below!
432-
433430
/// Returns this object as an [`Any`] trait object
434431
fn as_any(&self) -> &dyn Any;
435432

@@ -1059,6 +1056,7 @@ impl AliasedAggregateUDFImpl {
10591056
}
10601057
}
10611058

1059+
#[warn(clippy::missing_trait_methods)] // Delegates, so it should implement every single trait method
10621060
impl AggregateUDFImpl for AliasedAggregateUDFImpl {
10631061
fn as_any(&self) -> &dyn Any {
10641062
self
@@ -1084,6 +1082,32 @@ impl AggregateUDFImpl for AliasedAggregateUDFImpl {
10841082
&self.aliases
10851083
}
10861084

1085+
fn schema_name(&self, params: &AggregateFunctionParams) -> Result<String> {
1086+
self.inner.schema_name(params)
1087+
}
1088+
1089+
fn human_display(&self, params: &AggregateFunctionParams) -> Result<String> {
1090+
self.inner.human_display(params)
1091+
}
1092+
1093+
fn window_function_schema_name(
1094+
&self,
1095+
params: &WindowFunctionParams,
1096+
) -> Result<String> {
1097+
self.inner.window_function_schema_name(params)
1098+
}
1099+
1100+
fn display_name(&self, params: &AggregateFunctionParams) -> Result<String> {
1101+
self.inner.display_name(params)
1102+
}
1103+
1104+
fn window_function_display_name(
1105+
&self,
1106+
params: &WindowFunctionParams,
1107+
) -> Result<String> {
1108+
self.inner.window_function_display_name(params)
1109+
}
1110+
10871111
fn state_fields(&self, args: StateFieldsArgs) -> Result<Vec<FieldRef>> {
10881112
self.inner.state_fields(args)
10891113
}
@@ -1138,12 +1162,40 @@ impl AggregateUDFImpl for AliasedAggregateUDFImpl {
11381162
self.inner.coerce_types(arg_types)
11391163
}
11401164

1141-
udf_equals_hash!(AggregateUDFImpl);
1165+
fn return_field(&self, arg_fields: &[FieldRef]) -> Result<FieldRef> {
1166+
self.inner.return_field(arg_fields)
1167+
}
1168+
1169+
fn is_nullable(&self) -> bool {
1170+
self.inner.is_nullable()
1171+
}
11421172

11431173
fn is_descending(&self) -> Option<bool> {
11441174
self.inner.is_descending()
11451175
}
11461176

1177+
fn value_from_stats(&self, statistics_args: &StatisticsArgs) -> Option<ScalarValue> {
1178+
self.inner.value_from_stats(statistics_args)
1179+
}
1180+
1181+
fn default_value(&self, data_type: &DataType) -> Result<ScalarValue> {
1182+
self.inner.default_value(data_type)
1183+
}
1184+
1185+
fn supports_null_handling_clause(&self) -> bool {
1186+
self.inner.supports_null_handling_clause()
1187+
}
1188+
1189+
fn is_ordered_set_aggregate(&self) -> bool {
1190+
self.inner.is_ordered_set_aggregate()
1191+
}
1192+
1193+
fn set_monotonicity(&self, data_type: &DataType) -> SetMonotonicity {
1194+
self.inner.set_monotonicity(data_type)
1195+
}
1196+
1197+
udf_equals_hash!(AggregateUDFImpl);
1198+
11471199
fn documentation(&self) -> Option<&Documentation> {
11481200
self.inner.documentation()
11491201
}

datafusion/expr/src/udf.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -416,9 +416,6 @@ pub struct ReturnFieldArgs<'a> {
416416
/// let expr = add_one.call(vec![col("a")]);
417417
/// ```
418418
pub trait ScalarUDFImpl: Debug + Send + Sync {
419-
// Note: When adding any methods (with default implementations), remember to add them also
420-
// into the AliasedScalarUDFImpl below!
421-
422419
/// Returns this object as an [`Any`] trait object
423420
fn as_any(&self) -> &dyn Any;
424421

@@ -765,6 +762,7 @@ impl AliasedScalarUDFImpl {
765762
}
766763
}
767764

765+
#[warn(clippy::missing_trait_methods)] // Delegates, so it should implement every single trait method
768766
impl ScalarUDFImpl for AliasedScalarUDFImpl {
769767
fn as_any(&self) -> &dyn Any {
770768
self
@@ -794,6 +792,11 @@ impl ScalarUDFImpl for AliasedScalarUDFImpl {
794792
self.inner.return_field_from_args(args)
795793
}
796794

795+
fn is_nullable(&self, args: &[Expr], schema: &dyn ExprSchema) -> bool {
796+
#[allow(deprecated)]
797+
self.inner.is_nullable(args, schema)
798+
}
799+
797800
fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result<ColumnarValue> {
798801
self.inner.invoke_with_args(args)
799802
}

datafusion/expr/src/udwf.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,6 @@ where
306306
/// .unwrap();
307307
/// ```
308308
pub trait WindowUDFImpl: Debug + Send + Sync {
309-
// Note: When adding any methods (with default implementations), remember to add them also
310-
// into the AliasedWindowUDFImpl below!
311-
312309
/// Returns this object as an [`Any`] trait object
313310
fn as_any(&self) -> &dyn Any;
314311

@@ -500,6 +497,7 @@ impl AliasedWindowUDFImpl {
500497
}
501498
}
502499

500+
#[warn(clippy::missing_trait_methods)] // Delegates, so it should implement every single trait method
503501
impl WindowUDFImpl for AliasedWindowUDFImpl {
504502
fn as_any(&self) -> &dyn Any {
505503
self
@@ -549,6 +547,10 @@ impl WindowUDFImpl for AliasedWindowUDFImpl {
549547
self.inner.coerce_types(arg_types)
550548
}
551549

550+
fn reverse_expr(&self) -> ReversedUDWF {
551+
self.inner.reverse_expr()
552+
}
553+
552554
fn documentation(&self) -> Option<&Documentation> {
553555
self.inner.documentation()
554556
}

0 commit comments

Comments
 (0)