Rachelint commented on PR #11261: URL: https://github.com/apache/datafusion/pull/11261#issuecomment-2220576084
> > Ok, been a bit busy the past couple of days, continue to read the related codes and think a relatively good way to solve this today... One simple way I could think to solve it temporarily for not blocking the progress of #10943 is that we just treat it a special case like `count`: > > https://github.com/apache/datafusion/blob/1e0c06e14ae821ac6aa344f8acb638431a898ae8/datafusion/core/src/physical_optimizer/aggregate_statistics.rs#L144 > > no worries -- there is a lot going on these days. > > I think your short term workaround sounds very clever to me 👍 I think it is fine to leave figuring out the right general API to add to AggregateUdfImpl as a future project (my rationale being that special casing "max" / "min" is no worse than hard coding the `Min` and `Max` types, and unblocks the conversion to AggregateUDF) :smile: 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 to udf with a same name to overwrite the built-ins (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. -- 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]
