Rachelint commented on PR #11261: URL: https://github.com/apache/datafusion/pull/11261#issuecomment-2221773014
> > 😄 Thanks, I just impl the short term workaround now. But still worry about using the function name only to identify the built-in `min/max/count` in the optimizer. Assume that user define a udaf with a same name to overwrite the built-in (seems we can do it?). And the logic in user's `min/max/count` is totally different with the built-in. However the optimizer is unable to know the `min/max/count` is not the built-in(optimizer just identify by name), and do the mistaken optmization in result. > > I agree -- it will not be correct. I think that is why we eventually need to move the logic out of the optimizer and into the AggregateUDFImpl itself. 🤔 Seems indeed necessary to do that... 😄 Have finished the short term solution here. I originally want to avoid `downcast_ref::<AggregateFunctionExpr>()`, but found if we do that, we should add two functions `is_distinct` and `function_name` to `AggregateExpr` before. I am not sure if it is ok to add them... so I just impl with `downcast_ref::<AggregateFunctionExpr>()` now... -- 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
