alamb commented on issue #23814: URL: https://github.com/apache/datafusion/issues/23814#issuecomment-5073565010
> This all sounds right to me. Re. `apply_expressions`, most of the reasons for the [revert PR](https://github.com/apache/datafusion/pull/22437) are still true today > > * the method isn't called from vanilla datafusion Yes -- I think we should figure out some way to fix this -- I can't imagine there is no way we can't refactor some of DataFusion to use the API so that it will > * it cannot not have a default implementation, so every ExecutionPlan is forced to implement it Yes this is unfortunate but I don't see any way around it. It also moves us towards less downcasing (and thus more generic optimzer rules) > * it's tricky to implement because it involves tree node recursion and the behavior of the method can change often (ex. different expressions will be traversed depending on if its called before or after the filter pushdown optimizer pass I think it would make more sense if `apply_expressions` only walks the root of each expression (and any recursive tree walking is left to the caller if they want that) Perhaps we can handle this with appropriate documentation What I would suggest to make progress is finding some spot in the DataFusion code that could be rewritten to use apply_expressions -- 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]
