alamb commented on issue #10074: URL: https://github.com/apache/datafusion/issues/10074#issuecomment-2077439515
🤔 it seems like the core challenge is that the implementation of the aggregate functions -- specifically https://github.com/apache/arrow-datafusion/blob/671cef85c550969ab2c86d644968a048cb181c0c/datafusion/functions-aggregate/src/first_last.rs#L102-L117 need to have some way to create a `PhysicalExpr` without directly depending on `create_physical_expression` Rather than moving `create_physical_expression` around so the aggregtes could have a direct dependency (which I think is going to be tough to do without creating a cycle as create_physical_expression ), what about passing in a callback ? Something like ```rust pub struct AccumulatorArgs<'a> { ... /// Function for converting Exprs to PhysicalExprs, if needed create_physical_expression: &dyn Fn(&Expr) -> Result<Arc<dyn PhysicalExpr>>; ``` -- 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