alamb commented on code in PR #9874:
URL: https://github.com/apache/arrow-datafusion/pull/9874#discussion_r1549419111
##########
datafusion-examples/examples/advanced_udaf.rs:
##########
@@ -85,13 +87,21 @@ impl AggregateUDFImpl for GeoMeanUdaf {
/// is supported, DataFusion will use this row oriented
/// accumulator when the aggregate function is used as a window function
/// or when there are only aggregates (no GROUP BY columns) in the plan.
- fn accumulator(&self, _arg: &DataType) -> Result<Box<dyn Accumulator>> {
+ fn accumulator(&self, _acc_args: AccumulatorArgs) -> Result<Box<dyn
Accumulator>> {
Review Comment:
I was thinking about the impact on this API for UDAF writers last night.
Specifically, about the many existing UDAFs that exist / will exist at the
time this change gets released and on the first time people encounter / try to
use this API. i think the args with datatypes is much easier to use (and has
less mental gymnastics to use). Thus I am going to propose an easier / beginner
API for this that will require fewer changes to existing UDAFs and will be
easier to use for first timers
--
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]
