ozankabak commented on issue #8913: URL: https://github.com/apache/arrow-datafusion/issues/8913#issuecomment-1902053771
> This is actually interresting use case and I don't see that requirement in the current codebase. Are you adding this optimization in https://github.com/apache/arrow-datafusion/pull/8817 to somewhere? Use cases where composability/reusability is critical are not yet in the current codebase (upstream) yet but at least we at Synnada have downstream use case. > Also, I'm not sure how often is this required so it might not be a good idea to transform all usecases to this direction. Having worked with most of the physical optimization rules for a long time now, I think I can safely that this is necessary. As I mentioned before, it is also on our roadmap to contribute more physical optimizations upstream and they need this kind of composability/reusability. > I don't think that code duplication is the major issue here. I think the main issue with the current 11 derived trees (and also with the proposed generic `PlanContext` and `ExprContext`) is that the algorithms they use need to maintain both the base tree and the derived tree and keeping them in sync. This is not really true though -- once you have the generics, you do not need to maintain anything with regards to `apply_children`, `map_children` and others. All in all, the composability/reusability we will get via #8817 is really critical to certain use cases and upcoming contributions. Let's get that in and then let's discuss refining the `transform` signatures and fixing inefficiencies pertaining to `Recursion` enums. I actually like some of your core ideas but I get the sense that you may not have the full picture on this part of the code, its possible downstream uses and possible future extensions. -- 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]
