findepi commented on code in PR #16625:
URL: https://github.com/apache/datafusion/pull/16625#discussion_r2177026287
##########
datafusion/physical-expr/src/aggregate.rs:
##########
@@ -575,18 +576,10 @@ impl AggregateFunctionExpr {
ReversedUDAF::NotSupported => None,
ReversedUDAF::Identical => Some(self.clone()),
ReversedUDAF::Reversed(reverse_udf) => {
- let mut name = self.name().to_string();
Review Comment:
Thanks!
I agree this code was deliberate & nice. I hope we don't parse those names
though.
If there is a better solution to agg reverse causing failures
(https://github.com/apache/datafusion/pull/16625#discussion_r2176919070), let
me know. I can also drop this fix, I don't like it too.
Alternatively to the fix, I can block reversing for beneficial functions and
thus hide the problem for now. Would it be preferred for this PR?
--
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
