timsaucer commented on code in PR #15911:
URL: https://github.com/apache/datafusion/pull/15911#discussion_r2474355513
##########
datafusion/expr/src/udaf.rs:
##########
@@ -674,6 +681,31 @@ pub trait AggregateUDFImpl: Debug + Send + Sync {
/// the arguments
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType>;
+ /// What type will be returned by this function, given the arguments?
+ ///
+ /// By default, this function calls [`Self::return_type`] with the
+ /// types of each argument.
+ ///
+ /// # Notes
+ ///
+ /// Most UDFs should implement [`Self::return_type`] and not this
+ /// function as the output type for most functions only depends on the
types
+ /// of their inputs (e.g. `sum(f64)` is always `f64`).
+ ///
+ /// This function can be used for more advanced cases such as:
+ ///
+ /// 1. specifying nullability
+ /// 2. return types based on the **values** of the arguments (rather than
+ /// their **types**.
Review Comment:
On reading it back months later this docstring looks incorrect. The idea is
there in the Scalar UDF but you are correct that it is not handled in the same
way in aggregates here. In Scalar UDFs when we call the similar function we
pass along `ReturnFieldArgs` not simply `&[FieldRef]`. There we know when we
have scalar values that can modify the return.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
