goldmedal commented on issue #10870: URL: https://github.com/apache/datafusion/issues/10870#issuecomment-2161093938
Hi @jayzhan211, Continuing from our previous discussion in https://github.com/apache/datafusion/issues/10838#issuecomment-2156459652, I believe there are still some issues. We create `AccumulatorArgs` in https://github.com/apache/datafusion/blob/main/datafusion/physical-expr-common/src/aggregate/mod.rs#L287. However, `AggregateFunctionExpr` only takes `args` that are `PhysicalExpr`. I think `AccumulatorArgs` is a logical plan struct that should be placed in `datafusion-expr`. It can't access `PhysicalExpr`. Curiously, is it possible to convert `PhysicalExpr` back to `Expr`? If not, I plan to pass the original `Expr` to `AggregateFunctionExpr` (just like `sort_exprs: Vec<Expr>`), then pass it to `AccumulatorArgs`. Does this make sense? What do you think? Thanks -- 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
