berkaysynnada commented on PR #15566: URL: https://github.com/apache/datafusion/pull/15566#issuecomment-2812881155
> > * Having 2 parameters for plans seems very strange. On the other hand, removing it forces us to make deep copies. However, when I look the copied structs, it doesn't seem to have big deal as it's done only one time. Perhaps we can figure out a way avoiding those 2 defects at the same time (via another trait, or another API's). I'm open to discuss this > > While I am worried about the `ExecutionPlan`s we have in DataFusion but even more concerning to me is the one's we don't see: custom user plans which might be quite expensive to clone. And even if it's not a big impact for this rule if we add more and more rules each with multiple recursions it's going to be a non-trivial overhead. Especially since DataFusion doesn't support prepared statements or anything like that, it has to re-plan every time a query is run. If I have to select one option, I prefer the simple API honestly, and take the cloning cost as it's not a big deal for now at least. Let's listen some other people's opinion on this? For the multiple recursion issue, if we are stick to dropping plans which the filter does't reach to the source, O(N^2) is a must -- 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
