crepererum commented on PR #7775: URL: https://github.com/apache/arrow-datafusion/pull/7775#issuecomment-1757548567
> How about `Arc<RefCell<LogicalPlan>>`? We can return a new plan by changing the original plan by `plan.borrow_mut()`. I think `RefCell` and interior mutability make code VERY hard to understand and debug. Rust has very clear API types that lets you pass by value, pass by ref or pass by mutable ref and they all mean different things. Hacking around it for such a fundamental type is IMHO a no-go. I think `optimize` should be: ```rust pub fn optimize(&self, plan: Arc<LogicalPlan>) -> Result<Arc<LogicalPlan>> ``` If the plan stays the same, you can just pass through the `Arc`. If not, you can create a new one. All child nodes can easily be cloned (since they are `Arc`ed, at least after the change), and all other metadata that is attached to nodes should either be `Arc`ed as well or should be cheap to clone. -- 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]
