alamb commented on issue #9637: URL: https://github.com/apache/arrow-datafusion/issues/9637#issuecomment-2002184743
> I have two idea before deep diving to the code I think 2 sounds interesting Another think I was thinking was something like `LogicalPlan::rewrite(mut self, rewriter)` I think that along with [`Arc::try_unwrap`](https://doc.rust-lang.org/std/sync/struct.Arc.html#method.try_unwrap) could be used to minimize the places where cloning was actually needed Maybe we can prototype with ```rust impl OptimizerRule { ... /// does this rule support rewriting owned plans? fn supports_owned(&self) -> bool { return false } /// if supports_owned returns true, calls try_optimize_owned fn try_optimize_owned( &self, plan: LogicalPlan, config: &dyn OptimizerConfig ) -> Result<Transformed<LogicalPlan>, DataFusionError> { not_implemented_err!("try_optimized_owned is not implemented for this rule") } ... ``` And then play around with the code that calls `try_optimize` here https://github.com/apache/arrow-datafusion/blob/f4107d47bb4c0260d301294ddfc7c67d96574636/datafusion/optimizer/src/optimizer.rs#L360-L368 to try and use the `try_optimize_owned` API (without having to rewrite all the optimizers) for `SimplifyExprs` If we could show a significant performance improvement for the `sql_planner` benchmarks then I think it would be worth spending time reworking the other APIs -- 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]
