mingmwang commented on code in PR #5630:
URL: https://github.com/apache/arrow-datafusion/pull/5630#discussion_r1141582958
##########
datafusion/core/src/physical_plan/tree_node/mod.rs:
##########
@@ -182,27 +202,59 @@ pub trait TreeNodeRewritable: Clone {
F: FnMut(Self) -> Result<Self>;
}
-/// Trait for potentially recursively transform an [`TreeNodeRewritable`] node
-/// tree. When passed to `TreeNodeRewritable::transform_using`,
`TreeNodeRewriter::mutate` is
+/// Implements the [visitor
+/// pattern](https://en.wikipedia.org/wiki/Visitor_pattern) for recursively
walking [`TreeNode`]s.
+///
+/// [`TreeNodeVisitor`] allows keeping the algorithms
+/// separate from the code to traverse the structure of the `TreeNode`
+/// tree and makes it easier to add new types of tree node and
+/// algorithms by.
+///
+/// When passed to[`TreeNode::accept`], [`TreeNode::pre_visit`]
+/// and [`TreeNode::post_visit`] are invoked recursively
+/// on an node tree.
+///
+/// If an [`Err`] result is returned, recursion is stopped
+/// immediately.
+///
+/// If [`Recursion::Stop`] is returned on a call to pre_visit, no
+/// children of that tree node are visited, nor is post_visit
+/// called on that tree node
+pub trait TreeNodeVisitor: Sized {
+ /// The node type which is visitable.
+ type N: TreeNode;
+
+ /// Invoked before any children of `node` are visited.
+ fn pre_visit(&mut self, node: &Self::N) -> Result<Recursion>;
+
+ /// Invoked after all children of `node` are visited. Default
+ /// implementation does nothing.
+ fn post_visit(&mut self, _node: &Self::N) -> Result<Recursion> {
+ Ok(Recursion::Continue)
+ }
+}
+
+/// Trait for potentially recursively transform an [`TreeNode`] node
+/// tree. When passed to `TreeNode::transform_using`,
`TreeNodeRewriter::mutate` is
/// invoked recursively on all nodes of a tree.
Review Comment:
There is `transform_using` method any more. Please fix the comment.
##########
datafusion/core/src/physical_plan/tree_node/mod.rs:
##########
@@ -182,27 +202,59 @@ pub trait TreeNodeRewritable: Clone {
F: FnMut(Self) -> Result<Self>;
}
-/// Trait for potentially recursively transform an [`TreeNodeRewritable`] node
-/// tree. When passed to `TreeNodeRewritable::transform_using`,
`TreeNodeRewriter::mutate` is
+/// Implements the [visitor
+/// pattern](https://en.wikipedia.org/wiki/Visitor_pattern) for recursively
walking [`TreeNode`]s.
+///
+/// [`TreeNodeVisitor`] allows keeping the algorithms
+/// separate from the code to traverse the structure of the `TreeNode`
+/// tree and makes it easier to add new types of tree node and
+/// algorithms by.
+///
+/// When passed to[`TreeNode::accept`], [`TreeNode::pre_visit`]
+/// and [`TreeNode::post_visit`] are invoked recursively
+/// on an node tree.
+///
+/// If an [`Err`] result is returned, recursion is stopped
+/// immediately.
+///
+/// If [`Recursion::Stop`] is returned on a call to pre_visit, no
+/// children of that tree node are visited, nor is post_visit
+/// called on that tree node
+pub trait TreeNodeVisitor: Sized {
+ /// The node type which is visitable.
+ type N: TreeNode;
+
+ /// Invoked before any children of `node` are visited.
+ fn pre_visit(&mut self, node: &Self::N) -> Result<Recursion>;
+
+ /// Invoked after all children of `node` are visited. Default
+ /// implementation does nothing.
+ fn post_visit(&mut self, _node: &Self::N) -> Result<Recursion> {
+ Ok(Recursion::Continue)
+ }
+}
+
+/// Trait for potentially recursively transform an [`TreeNode`] node
+/// tree. When passed to `TreeNode::transform_using`,
`TreeNodeRewriter::mutate` is
/// invoked recursively on all nodes of a tree.
Review Comment:
There is no `transform_using` method any more. Please fix the comment.
--
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]
