alamb commented on code in PR #9249:
URL: https://github.com/apache/arrow-datafusion/pull/9249#discussion_r1504090164
##########
datafusion/expr/src/udaf.rs:
##########
@@ -255,15 +261,20 @@ pub trait AggregateUDFImpl: Debug + Send + Sync {
/// Return a new [`Accumulator`] that aggregates values for a specific
/// group during query execution.
- fn accumulator(&self, arg: &DataType) -> Result<Box<dyn Accumulator>>;
+ fn accumulator(
Review Comment:
I think this API:
1. Doesn't need an owned Schema (passing in a `Schema` requires cloning
which I think we can avoid
2. I am not sure why it needs an `Option` (why is it not always passed in)
3. Should have the documentation updated to explain what sort_exprs are and
what schema is
##########
datafusion/expr/src/expr_fn.rs:
##########
@@ -1073,6 +1075,34 @@ pub fn create_udaf(
))
}
+/// Creates a new UDAF with a specific signature, state type and return type.
Review Comment:
Passing in the schema is a good call
##########
datafusion/core/tests/user_defined/user_defined_aggregates.rs:
##########
@@ -659,7 +765,12 @@ impl AggregateUDFImpl for TestGroupsAccumulator {
Ok(DataType::UInt64)
}
- fn accumulator(&self, _arg: &DataType) -> Result<Box<dyn Accumulator>> {
+ fn accumulator(
+ &self,
+ _arg: &DataType,
+ _sort_exprs: Vec<Expr>,
+ _schmea: Option<Schema>,
Review Comment:
```suggestion
_schema: Option<Schema>,
```
--
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]
