alamb commented on issue #4628: URL: https://github.com/apache/arrow-datafusion/issues/4628#issuecomment-1454895268
> I think you would want to make try_optimize take ownership not a borrow, and then return it I think there are cases (like deciding when a fixed point is reached) where the caller wants to distinguish between no more optimization and a new plan. However, now that LogicalPlan supports `PartialEq` I think maybe we could just use that to check if any changes were made https://github.com/apache/arrow-datafusion/blob/c37ddf72ec539bd39cce0dd4ff38db2e36ddb55f/datafusion/expr/src/logical_plan/plan.rs#L52 So the signature maybe could be ```rust /// Try and rewrite `plan` to an optimized form fn try_optimize( &self, plan: LogicalPlan, config: &dyn OptimizerConfig, ) -> Result<LogicalPlan>; ```` 🤔 -- 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]
