alamb commented on issue #11810: URL: https://github.com/apache/datafusion/issues/11810#issuecomment-2296941277
> Now all aggr functions have been migrated to UDAF. The AggregateExpr trait just has one implementation. This seems like a compelling reason to remove `AggregateExpr` (it is unecessary and makes the code more complicated) > It is not clear to me whether it is a good idea. Maybe hold on and review it after the refactor https://github.com/apache/datafusion/issues/11359 ? Now that https://github.com/apache/datafusion/issues/11359 is done, @jayzhan211 do you have any additional thoughts? I agree with your comment on https://github.com/apache/datafusion/pull/11989/files#r1720745280 that there is room for improvement here -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org