adriangb commented on code in PR #22298:
URL: https://github.com/apache/datafusion/pull/22298#discussion_r3276294474
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -360,6 +366,121 @@ impl TreeNodeRewriter for Rewriter<'_> {
}
}
+/// Applies `f` to each child (input) of `plan` in place, using
+/// [`Arc::make_mut`] for copy-on-write semantics on `Arc<LogicalPlan>`
+/// children. When the `Arc` refcount is 1 (the common case here)
+/// `Arc::make_mut` hands out a `&mut` without cloning; when it is >1 the
+/// inner value is cloned first.
+///
+/// Returns `Ok(true)` if any child was modified by `f`.
+///
+/// This is deliberately private to the optimizer rather than a method on
+/// [`LogicalPlan`]: it is an implementation detail of in-place rewriting, and
+/// the `Arc::make_mut` approach does not generalize to the other tree types
+/// (`Expr` children are `Box`ed; `PhysicalExpr`/`ExecutionPlan` children are
+/// `Arc<dyn _>`, which `Arc::make_mut` cannot handle). If `TreeNode` ever
+/// grows an in-place traversal this logic can move there.
+///
+/// # Error semantics
+///
+/// If `f` returns `Err` for a child, that error is returned immediately;
+/// children visited earlier keep whatever modifications `f` already applied
+/// to them — they are **not** rolled back.
+fn map_children_mut<F: FnMut(&mut LogicalPlan) -> Result<bool>>(
Review Comment:
Yes I'll add an issue
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
