xudong963 commented on code in PR #14650:
URL: https://github.com/apache/datafusion/pull/14650#discussion_r1957131431
##########
datafusion/physical-plan/src/tree_node.rs:
##########
@@ -42,6 +42,8 @@ impl DynTreeNode for dyn ExecutionPlan {
/// A node object beneficial for writing optimizer rules, encapsulating an
[`ExecutionPlan`] node with a payload.
/// Since there are two ways to access child plans—directly from the plan and
through child nodes—it's recommended
/// to perform mutable operations via [`Self::update_plan_from_children`].
+/// After update `children`, please do the sync updating for `plan`'s children.
Review Comment:
Yes, I've tried this way to make the operation "atomic", but I found it hard
to change, most places acquire `mut self` for `PlanContext`.
I have another way to avoid the mind burden. Currrently, we have two
parallel tree structures:
- The ExecutionPlan tree (through plan field and its children)
- The PlanContext tree (through children field)
I'm trying to make them single.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
