irenjj commented on PR #15253: URL: https://github.com/apache/datafusion/pull/15253#issuecomment-2761472370
> @irenjj `AggregateFunctionExpr` has `with_new_expressions()` API. As datafusion hasn't implemented it yet, you didn't have difficulty rewriting the `human_display` according to the new expressions. Could you suggest a way to update `human_readable` field for the implementers of `with_new_expressions()`? I believe that the `human_display` in `AggregateFunctionExpr` is constructed during the logical plan phase based on the available information at that time (and the `name` in `AggregateFunctionExpr` is similar). On the other hand, `with_new_expressions()` replaces the `args` in `AggregateFunctionExpr` during the physical plan phase (perhaps related to the optimizer?). It's perfectly fine to keep `human_display` unchanged when implementing `with_new_expressions()` because the `name` used in the original `indented explain` is actually quite similar to `human_display`. Perhaps what we should really focus on is ensuring that the explain output correctly displays the `name` of the `expressions` after `with_new_expressions()` has been applied. -- 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
