peter-toth commented on PR #7942: URL: https://github.com/apache/arrow-datafusion/pull/7942#issuecomment-1864759838
> I like where this is going 🚀 > > I suggest to also add some benchmarking. We could take for example TCP-H and TCP-DS (which we already have in the benchmarks / tests) and benchmark the time it takes to plan/optimize the queries rather than execute them. It seems it might not be much work adding an option to the benchmark code to only perform the planning rather than executing the queries. I like this idea, but I'm not sure that this PR itself can bring much improvement yet. This PR only refactors a few transform/rewrite operations but the old ones are still kept and used at many places. Also, some trees like `LogicalPlan` uses `Arc`s and their new in place mutation (`transform_children()` in this PR: https://github.com/apache/arrow-datafusion/pull/7942/files#diff-9619441d9605f143a911319cea75ae5192e6c5b17acfcbc17a3c73a9e32a8e61R62-R80) is not yet better than their old `map_children()` (https://github.com/apache/arrow-datafusion/pull/7942/files#diff-9619441d9605f143a911319cea75ae5192e6c5b17acfcbc17a3c73a9e32a8e61R39) is. Actually I'm not sure yet it's possible to do in place mutation on `Arc`s at all. BTW, anyone can explain me why are this difference between `TreeNode`s in Datafusion? Why do some of them use `Box`s but others use `Arc`s? -- 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]
