peter-toth commented on code in PR #10035:
URL:
https://github.com/apache/arrow-datafusion/pull/10035#discussion_r1567296656
##########
datafusion/common/src/tree_node.rs:
##########
@@ -43,22 +42,80 @@ macro_rules! handle_transform_recursion_up {
}};
}
-/// Defines a visitable and rewriteable tree node. This trait is implemented
-/// for plans ([`ExecutionPlan`] and [`LogicalPlan`]) as well as expression
-/// trees ([`PhysicalExpr`], [`Expr`]) in DataFusion.
+/// API for inspecting and rewriting tree data structures.
+///
+/// The `TreeNode` API is used to express algorithms separately from traversing
+/// the structure of `TreeNode`s, avoiding substantial code duplication.
+///
+/// This trait is implemented for plans ([`ExecutionPlan`] and [`LogicalPlan`])
Review Comment:
Probably these 4 are the most important ones, but there are a few other
implementations like `ConcreteTreeNode`s e.g. `PlanContext` and `ExprContext`.
##########
datafusion/common/src/tree_node.rs:
##########
@@ -93,12 +142,28 @@ pub trait TreeNode: Sized {
.visit_parent(|| visitor.f_up(self))
}
- /// Implements the [visitor
pattern](https://en.wikipedia.org/wiki/Visitor_pattern) for
- /// recursively transforming [`TreeNode`]s.
+ /// Rewrite the tree node with a [`TreeNodeRewriter`], performing a
+ /// depth-first walk of the node and its children.
+ ///
+ /// [`TreeNodeRewriter::f_down()`] is called in top-down order (before
+ /// children are visited), [`TreeNodeRewriter::f_up()`] is called in
+ /// bottom-up order (after children are visited).
///
- /// See also:
- /// * [`Self::visit`] for inspecting (without modification) `TreeNode`s
+ /// Note: If using the default [`TreeNodeRewriter::f_up`] that does
nothing,
Review Comment:
This is true for the other way around too. If `TreeNodeRewriter::f_down()`
does nothing then `transform_up()` is easier to use.
--
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]
