From d8136c16a269d8e3c7e47d3984abf9d00dd6fe96 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 10 Apr 2024 17:29:22 -0400 Subject: [PATCH 1/7] Improve documentation on `TreeNode` --- datafusion/common/src/tree_node.rs | 130 ++++++++++++++++++----------- 1 file changed, 79 insertions(+), 51 deletions(-) diff --git a/datafusion/common/src/tree_node.rs b/datafusion/common/src/tree_node.rs index dff22d4959582..c6d7fe26c468c 100644 --- a/datafusion/common/src/tree_node.rs +++ b/datafusion/common/src/tree_node.rs @@ -15,8 +15,7 @@ // specific language governing permissions and limitations // under the License. -//! This module provides common traits for visiting or rewriting tree -//! data structures easily. +//! [`TreeNode`] for visiting and rewriting expression and plan trees use std::sync::Arc; @@ -43,9 +42,24 @@ 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 visiting (read only) and rewriting tree data structures . +/// +/// Note: this trait is implemented for plans ([`ExecutionPlan`] and +/// [`LogicalPlan`]) as well as expression trees ([`PhysicalExpr`], [`Expr`]). +/// +/// # Common APIs: +/// * [`Self::apply`] and [`Self::visit`] for inspecting (without modification) borrowed nodes +/// * [`Self::transform`] and [`Self::rewrite`] for rewriting owned nodes +/// +/// # Terminology +/// The following terms are used in this trait +/// +/// * `f_down`: Invoked before any children of the current node are visited. +/// * `f_up`: Invoked after all children of the current node are visited. +/// * `f`: closure that is applied to the current node. +/// * `map_*`: applies a transformation to rewrite owned nodes +/// * `apply_*`: invokes a function on borrowed nodes +/// * `transform_`: applies a transformation to rewrite owned nodes /// /// /// [`ExecutionPlan`]: https://docs.rs/datafusion/latest/datafusion/physical_plan/trait.ExecutionPlan.html @@ -53,7 +67,7 @@ macro_rules! handle_transform_recursion_up { /// [`LogicalPlan`]: https://docs.rs/datafusion-expr/latest/datafusion_expr/logical_plan/enum.LogicalPlan.html /// [`Expr`]: https://docs.rs/datafusion-expr/latest/datafusion_expr/expr/enum.Expr.html pub trait TreeNode: Sized { - /// Visit the tree node using the given [`TreeNodeVisitor`], performing a + /// Visit the tree node with a [`TreeNodeVisitor`], performing a /// depth-first walk of the node and its children. /// /// See also: @@ -93,8 +107,8 @@ 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. /// /// See also: /// * [`Self::visit`] for inspecting (without modification) `TreeNode`s @@ -129,12 +143,11 @@ pub trait TreeNode: Sized { }) } - /// Applies `f` to the node and its children. `f` is applied in a pre-order - /// way, and it is controlled by [`TreeNodeRecursion`], which means result - /// of the `f` on a node can cause an early return. + /// Applies `f` to the node then each of its children, recursively (a + /// top-down, pre-order traversal). /// - /// The `f` closure can be used to collect some information from tree nodes - /// or run a check on the tree. + /// The return [`TreeNodeRecursion`] controls the recursion and can cause + /// an early return. fn apply Result>( &self, f: &mut F, @@ -142,9 +155,15 @@ pub trait TreeNode: Sized { f(self)?.visit_children(|| self.apply_children(|c| c.apply(f))) } - /// Convenience utility for writing optimizer rules: Recursively apply the - /// given function `f` to the tree in a bottom-up (post-order) fashion. When - /// `f` does not apply to a given node, it is left unchanged. + /// Recursively rewrite the node's children and then the node using `f` + /// (a bottom-up post-order traversal). + /// + /// A synonym of [`Self::transform_up`]. `f` is applied to the nodes + /// children first, and then to the node itself. See [`Self::transform_down`] + /// top-down (pre-order) traversal. + /// + /// Use [`TreeNodeRewriter`] for an API that supports transformations both + /// down and up. fn transform Result>>( self, f: &F, @@ -152,9 +171,13 @@ pub trait TreeNode: Sized { self.transform_up(f) } - /// Convenience utility for writing optimizer rules: Recursively apply the - /// given function `f` to a node and then to its children (pre-order traversal). - /// When `f` does not apply to a given node, it is left unchanged. + /// Recursively rewrite the tree using `f` in a top-down (pre-order) + /// fashion. + /// + /// `f` is applied to the nodes first, and then its children. + /// + /// Use [`TreeNodeRewriter`] for an API that supports transformations both + /// down and up the tree. fn transform_down Result>>( self, f: &F, @@ -162,9 +185,7 @@ pub trait TreeNode: Sized { handle_transform_recursion_down!(f(self), |c| c.transform_down(f)) } - /// Convenience utility for writing optimizer rules: Recursively apply the - /// given mutable function `f` to a node and then to its children (pre-order - /// traversal). When `f` does not apply to a given node, it is left unchanged. + /// Same as [`Self::transform_down`] but with a mutable closure. fn transform_down_mut Result>>( self, f: &mut F, @@ -172,10 +193,13 @@ pub trait TreeNode: Sized { handle_transform_recursion_down!(f(self), |c| c.transform_down_mut(f)) } - /// Convenience utility for writing optimizer rules: Recursively apply the - /// given function `f` to all children of a node, and then to the node itself - /// (post-order traversal). When `f` does not apply to a given node, it is - /// left unchanged. + /// Recursively rewrite the node using `f` in a bottom-up (post-order) + /// fashion. + /// + /// `f` is applied to children first, and then to the node itself. + /// + /// Use [`TreeNodeRewriter`] for an API that supports transformations both + /// down and up the tree. fn transform_up Result>>( self, f: &F, @@ -183,10 +207,8 @@ pub trait TreeNode: Sized { handle_transform_recursion_up!(self, |c| c.transform_up(f), f) } - /// Convenience utility for writing optimizer rules: Recursively apply the - /// given mutable function `f` to all children of a node, and then to the - /// node itself (post-order traversal). When `f` does not apply to a given - /// node, it is left unchanged. + /// Same as [`Self::transform_up`] but with a mutable closure. + fn transform_up_mut Result>>( self, f: &mut F, @@ -194,14 +216,14 @@ pub trait TreeNode: Sized { handle_transform_recursion_up!(self, |c| c.transform_up_mut(f), f) } - /// Transforms the tree using `f_down` while traversing the tree top-down + /// Transforms the node using `f_down` while traversing the tree top-down /// (pre-order), and using `f_up` while traversing the tree bottom-up /// (post-order). /// /// Use this method if you want to start the `f_up` process right where `f_down` jumps. /// This can make the whole process faster by reducing the number of `f_up` steps. /// If you don't need this, it's just like using `transform_down_mut` followed by - /// `transform_up_mut` on the same tree. + /// `transform_up_mut` on the same node. /// /// Consider the following tree structure: /// ```text @@ -298,7 +320,7 @@ pub trait TreeNode: Sized { ) } - /// Returns true if `f` returns true for node in the tree. + /// Returns true if `f` returns true for any node in the tree. /// /// Stops recursion as soon as a matching node is found fn exists bool>(&self, mut f: F) -> bool { @@ -315,16 +337,21 @@ pub trait TreeNode: Sized { found } - /// Apply the closure `F` to the node's children. + /// Apply `f` to inspect node's children (but not the node itself) + /// + /// See [`Self::apply`] and [`Self::visit`] for APIs that include self. /// - /// See `mutate_children` for rewriting in place + /// See [`Self::map_children`] for rewriting in place fn apply_children Result>( &self, f: F, ) -> Result; - /// Apply transform `F` to potentially rewrite the node's children. Note - /// that the transform `F` might have a direction (pre-order or post-order). + /// Apply `f` to rewrite the node's children (but not the node itself). + /// + /// See [`Self::transform`] and [`Self::rewrite`] for APIs that include self + /// + /// See [`Self::apply_children`] for inspecting in place fn map_children Result>>( self, f: F, @@ -436,26 +463,26 @@ impl TreeNodeRecursion { } } -/// This struct is used by tree transformation APIs such as +/// Result of tree walk / transformation APIs +/// +/// API users control the transformation by returning: +/// - The resulting (possibly transformed) node, +/// - `transformed`: flag indicating whether any change was made to the node +/// - `tnr`: [`TreeNodeRecursion`] specifying how to proceed with the recursion. +/// +/// At the end of the transformation, the return value will contain: +/// - The final (possibly transformed) tree, +/// - `transformed`: flag indicating whether any change was made to the node +/// - `tnr`: [`TreeNodeRecursion`] specifying how the recursion ended. +/// +/// Example APIs: +/// - [`TreeNode`], /// - [`TreeNode::rewrite`], /// - [`TreeNode::transform_down`], /// - [`TreeNode::transform_down_mut`], /// - [`TreeNode::transform_up`], /// - [`TreeNode::transform_up_mut`], /// - [`TreeNode::transform_down_up`] -/// -/// to control the transformation and return the transformed result. -/// -/// Specifically, API users can provide transformation closures or [`TreeNodeRewriter`] -/// implementations to control the transformation by returning: -/// - The resulting (possibly transformed) node, -/// - A flag indicating whether any change was made to the node, and -/// - A flag specifying how to proceed with the recursion. -/// -/// At the end of the transformation, the return value will contain: -/// - The final (possibly transformed) tree, -/// - A flag indicating whether any change was made to the tree, and -/// - A flag specifying how the recursion ended. #[derive(PartialEq, Debug)] pub struct Transformed { pub data: T, @@ -637,6 +664,7 @@ impl TreeNodeIterator for I { /// Transformation helper to process a heterogeneous sequence of tree node containing /// expressions. +/// /// This macro is very similar to [TreeNodeIterator::map_until_stop_and_collect] to /// process nodes that are siblings, but it accepts an initial transformation (`F0`) and /// a sequence of pairs. Each pair is made of an expression (`EXPR`) and its From d0de24691c1f3fca08947ee3e3f9850ebc0e7b99 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 16 Apr 2024 07:36:41 -0400 Subject: [PATCH 2/7] Document inspecting, rewriting APIs. Add chart --- datafusion/common/src/tree_node.rs | 203 +++++++++++++++++++++-------- 1 file changed, 147 insertions(+), 56 deletions(-) diff --git a/datafusion/common/src/tree_node.rs b/datafusion/common/src/tree_node.rs index c6d7fe26c468c..b3c3b02641dc6 100644 --- a/datafusion/common/src/tree_node.rs +++ b/datafusion/common/src/tree_node.rs @@ -42,14 +42,47 @@ macro_rules! handle_transform_recursion_up { }}; } -/// API for visiting (read only) and rewriting tree data structures . +/// API for inspecting and rewriting tree data structures. /// -/// Note: this trait is implemented for plans ([`ExecutionPlan`] and -/// [`LogicalPlan`]) as well as expression trees ([`PhysicalExpr`], [`Expr`]). +/// The `TreeNode` API is used to express algorithms separately from traversing +/// the structure of `TreeNode`s, avoiding substantial code duplication. /// -/// # Common APIs: -/// * [`Self::apply`] and [`Self::visit`] for inspecting (without modification) borrowed nodes -/// * [`Self::transform`] and [`Self::rewrite`] for rewriting owned nodes +/// This trait is implemented for plans ([`ExecutionPlan`] and [`LogicalPlan`]) +/// and expression trees ([`PhysicalExpr`], [`Expr`]). +/// +/// # Overview +/// There are three categories of TreeNode APIs: +/// +/// 1. "Inspecting" APIs to traverse a tree of `&TreeNodes`: +/// [`apply`] and [`visit`]. +/// +/// 2. "Transforming" APIs that traverse and consume a tree of `TreeNode`s +/// producing possibly changed `TreeNode`s: [`transform`], [`transform_up`], +/// [`transform_down`], [`transform_down_up`], and [`rewrite`]. +/// +/// 3. Internal APIs used to implement the `TreeNode` API: [`apply_children`], +/// and [`map_children`]. +/// +/// | Traversal Order | Inspecting | Transforming | +/// | --- | --- | --- | +/// | top-down | [`apply`] | [`transform_down`]| +/// | bottom-up | | [`transform`] , [`transform_up`]| +/// | combined with separate `f_down` and `f_up` closures | | [`transform_down_up`] | +/// | combined with `f_down()` and `f_up()` in an object | [`visit`] | [`rewrite`] | +/// +/// **Note**:while there is currently no in-place mutation API that uses `&mut +/// TreeNode`, the transforming APIs are efficient and optimized to avoid +/// cloning. +/// +/// [`apply`]: Self::apply +/// [`visit`]: Self::visit +/// [`transform`]: Self::transform +/// [`transform_up`]: Self::transform_up +/// [`transform_down`]: Self::transform_down +/// [`transform_down_up`]: Self::transform_down_up +/// [`rewrite`]: Self::rewrite +/// [`apply_children`]: Self::apply_children +/// [`map_children`]: Self::map_children /// /// # Terminology /// The following terms are used in this trait @@ -70,9 +103,19 @@ pub trait TreeNode: Sized { /// Visit the tree node with a [`TreeNodeVisitor`], performing a /// depth-first walk of the node and its children. /// - /// See also: - /// * [`Self::rewrite`] to rewrite owned `TreeNode`s + /// [`TreeNodeVisitor::f_down()`] is called in top-down order (before + /// children are visited), [`TreeNodeVisitor::f_up()`] is called in + /// bottom-up order (after children are visited). + /// + /// # Return Value + /// The returns value specifies how the tree walk ended. + /// [`TreeNodeRecursion`] for details. + /// + /// # See Also: + /// * [`Self::apply`] for inspecting nodes with a closure + /// * [`Self::rewrite`] to rewrite owned `TreeNode`s /// + /// # Example /// Consider the following tree structure: /// ```text /// ParentNode @@ -89,14 +132,6 @@ pub trait TreeNode: Sized { /// TreeNodeVisitor::f_up(ChildNode2) /// TreeNodeVisitor::f_up(ParentNode) /// ``` - /// - /// See [`TreeNodeRecursion`] for more details on controlling the traversal. - /// - /// If [`TreeNodeVisitor::f_down()`] or [`TreeNodeVisitor::f_up()`] returns [`Err`], - /// the recursion stops immediately. - /// - /// If using the default [`TreeNodeVisitor::f_up`] that does nothing, consider using - /// [`Self::apply`]. fn visit>( &self, visitor: &mut V, @@ -110,9 +145,25 @@ pub trait TreeNode: Sized { /// Rewrite the tree node with a [`TreeNodeRewriter`], performing a /// depth-first walk of the node and its children. /// - /// See also: - /// * [`Self::visit`] for inspecting (without modification) `TreeNode`s + /// [`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). /// + /// Note: If using the default [`TreeNodeRewriter::f_up`] that does nothing, + /// consider using [`Self::transform_down`]. + /// + /// # Return Value + /// The returns value specifies how the tree walk should proceed. See + /// [`TreeNodeRecursion`] for details. If an [`Err`] is returned, the + /// recursion stops immediately. + /// + /// # See Also + /// * [`Self::visit`] for inspecting (without modification) `TreeNode`s + /// * [Self::transform_down_up] for a top-down (pre-order) traversal. + /// * [Self::transform_down] for a top-down (pre-order) traversal. + /// * [`Self::transform_up`] for a bottom-up (post-order) traversal. + /// + /// # Example /// Consider the following tree structure: /// ```text /// ParentNode @@ -129,11 +180,6 @@ pub trait TreeNode: Sized { /// TreeNodeRewriter::f_up(ChildNode2) /// TreeNodeRewriter::f_up(ParentNode) /// ``` - /// - /// See [`TreeNodeRecursion`] for more details on controlling the traversal. - /// - /// If [`TreeNodeVisitor::f_down()`] or [`TreeNodeVisitor::f_up()`] returns [`Err`], - /// the recursion stops immediately. fn rewrite>( self, rewriter: &mut R, @@ -148,6 +194,10 @@ pub trait TreeNode: Sized { /// /// The return [`TreeNodeRecursion`] controls the recursion and can cause /// an early return. + /// + /// # See Also + /// * [`Self::transform_down`] for the equivalent transformation API. + /// * [`Self::visit`] for both top-down and bottom up traversal. fn apply Result>( &self, f: &mut F, @@ -155,15 +205,10 @@ pub trait TreeNode: Sized { f(self)?.visit_children(|| self.apply_children(|c| c.apply(f))) } - /// Recursively rewrite the node's children and then the node using `f` + /// Recursively rewrite the node's children and then the node using `f` /// (a bottom-up post-order traversal). /// - /// A synonym of [`Self::transform_up`]. `f` is applied to the nodes - /// children first, and then to the node itself. See [`Self::transform_down`] - /// top-down (pre-order) traversal. - /// - /// Use [`TreeNodeRewriter`] for an API that supports transformations both - /// down and up. + /// A synonym of [`Self::transform_up`]. fn transform Result>>( self, f: &F, @@ -174,10 +219,12 @@ pub trait TreeNode: Sized { /// Recursively rewrite the tree using `f` in a top-down (pre-order) /// fashion. /// - /// `f` is applied to the nodes first, and then its children. + /// `f` is applied to the node first, and then its children. /// - /// Use [`TreeNodeRewriter`] for an API that supports transformations both - /// down and up the tree. + /// # See Also + /// * [`Self::transform_up`] for a bottom-up (post-order) traversal. + /// * [Self::transform_down_up] for a combined traversal with closures + /// * [`Self::rewrite`] for a combined traversal with a visitor fn transform_down Result>>( self, f: &F, @@ -196,10 +243,12 @@ pub trait TreeNode: Sized { /// Recursively rewrite the node using `f` in a bottom-up (post-order) /// fashion. /// - /// `f` is applied to children first, and then to the node itself. + /// `f` is applied to the node's children first, and then to the node itself. /// - /// Use [`TreeNodeRewriter`] for an API that supports transformations both - /// down and up the tree. + /// # See Also + /// * [`Self::transform_down`] top-down (pre-order) traversal. + /// * [Self::transform_down_up] for a combined traversal with closures + /// * [`Self::rewrite`] for a combined traversal with a visitor fn transform_up Result>>( self, f: &F, @@ -220,11 +269,17 @@ pub trait TreeNode: Sized { /// (pre-order), and using `f_up` while traversing the tree bottom-up /// (post-order). /// - /// Use this method if you want to start the `f_up` process right where `f_down` jumps. - /// This can make the whole process faster by reducing the number of `f_up` steps. - /// If you don't need this, it's just like using `transform_down_mut` followed by - /// `transform_up_mut` on the same node. + /// The method behaves the same as calling [`Self::transform_down`] followed + /// by [`Self::transform_up`] on the same node. Use this method if you want + /// to start the `f_up` process right where `f_down` jumps. This can make + /// the whole process faster by reducing the number of `f_up` steps. /// + /// # See Also + /// * [`Self::transform_up`] for a bottom-up (post-order) traversal. + /// * [Self::transform_down] for a top-down (pre-order) traversal. + /// * [`Self::rewrite`] for a combined traversal with a visitor + /// + /// # Example /// Consider the following tree structure: /// ```text /// ParentNode @@ -337,55 +392,91 @@ pub trait TreeNode: Sized { found } - /// Apply `f` to inspect node's children (but not the node itself) + /// Low-level API used to implement other APIs. + /// + /// If you want to implement the [`TreeNode`] trait for your own type, you + /// should implement this method and [`Self::map_children`]. /// - /// See [`Self::apply`] and [`Self::visit`] for APIs that include self. + /// Users should use one of the higher level APIs described on [`Self`]. /// - /// See [`Self::map_children`] for rewriting in place + /// Description: Apply `f` to inspect node's children (but not the node + /// itself). fn apply_children Result>( &self, f: F, ) -> Result; - /// Apply `f` to rewrite the node's children (but not the node itself). + /// Low-level API used to implement other APIs. /// - /// See [`Self::transform`] and [`Self::rewrite`] for APIs that include self + /// If you want to implement the [`TreeNode`] trait for your own type, you + /// should implement this method and [`Self::apply_children`]. /// - /// See [`Self::apply_children`] for inspecting in place + /// Users should use one of the higher level APIs described on [`Self`]. + /// + /// Description: Apply `f` to rewrite the node's children (but not the node itself). fn map_children Result>>( self, f: F, ) -> Result>; } -/// Implements the [visitor pattern](https://en.wikipedia.org/wiki/Visitor_pattern) -/// for recursively walking [`TreeNode`]s. +/// A [Visitor](https://en.wikipedia.org/wiki/Visitor_pattern) for recursively +/// inspecting [`TreeNode`]s via [`TreeNode::visit`]. /// -/// A [`TreeNodeVisitor`] allows one to express algorithms separately from the -/// code traversing the structure of the `TreeNode` tree, making it easier to -/// add new types of tree nodes and algorithms. +/// See [`TreeNode`] for more details on available APIs /// /// When passed to [`TreeNode::visit`], [`TreeNodeVisitor::f_down`] and /// [`TreeNodeVisitor::f_up`] are invoked recursively on the tree. /// See [`TreeNodeRecursion`] for more details on controlling the traversal. +/// +/// # Return Value +/// The returns value of `f_up` and `f_down` specifies how the tree walk should +/// proceed. See [`TreeNodeRecursion`] for details. If an [`Err`] is returned, +/// the recursion stops immediately. +/// +/// Note: If using the default implementations of [`TreeNodeVisitor::f_up`] or +/// [`TreeNodeVisitor::f_down`] that do nothing, consider using +/// [`TreeNode::apply`] instead. +/// +/// # See Also: +/// * [`TreeNode::rewrite`] to rewrite owned `TreeNode`s pub trait TreeNodeVisitor: Sized { /// The node type which is visitable. type Node: TreeNode; - /// Invoked before any children of `node` are visited. - /// Default implementation simply continues the recursion. + /// Invoked while traversing down the tree, before any children are visited. + /// Default implementation continues the recursion. fn f_down(&mut self, _node: &Self::Node) -> Result { Ok(TreeNodeRecursion::Continue) } - /// Invoked after all children of `node` are visited. - /// Default implementation simply continues the recursion. + /// Invoked while traversing up the tree after children are visited. Default + /// implementation continues the recursion. fn f_up(&mut self, _node: &Self::Node) -> Result { Ok(TreeNodeRecursion::Continue) } } -/// Trait for potentially recursively transforming a tree of [`TreeNode`]s. +/// A [Visitor](https://en.wikipedia.org/wiki/Visitor_pattern) for recursively +/// rewriting [`TreeNode`]s via [`TreeNode::rewrite`]. +/// +/// See [`TreeNode`] for more details on available APIs +/// +/// When passed to [`TreeNode::rewrite`], [`TreeNodeRewriter::f_down`] and +/// [`TreeNodeRewriter::f_up`] are invoked recursively on the tree. +/// See [`TreeNodeRecursion`] for more details on controlling the traversal. +/// +/// # Return Value +/// The returns value of `f_up` and `f_down` specifies how the tree walk should +/// proceed. See [`TreeNodeRecursion`] for details. If an [`Err`] is returned, +/// the recursion stops immediately. +/// +/// Note: If using the default implementations of [`TreeNodeRewriter::f_up`] or +/// [`TreeNodeRewriter::f_down`] that do nothing, consider using +/// [`TreeNode::transform_up`] or [`TreeNode::transform_down`] instead. +/// +/// # See Also: +/// * [`TreeNode::visit`] to inspect borrowed `TreeNode`s pub trait TreeNodeRewriter: Sized { /// The node type which is rewritable. type Node: TreeNode; From a79a28325f1d3d949a4646503bf3f641f0e4f26f Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 16 Apr 2024 08:24:28 -0400 Subject: [PATCH 3/7] tweak --- datafusion/expr/src/logical_plan/plan.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index dbff5046013b7..2b2978f076afe 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -65,7 +65,7 @@ pub use datafusion_common::{JoinConstraint, JoinType}; /// from leaves up to the root to produce the query result. /// /// # See also: -/// * [`tree_node`]: visiting and rewriting API +/// * [`tree_node`]: To inspect and rewrite `LogicalPlan` trees /// /// [`tree_node`]: crate::logical_plan::tree_node #[derive(Clone, PartialEq, Eq, Hash)] From a0191d2ad7cf4987db719e490c63c41d44ca3539 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 17 Apr 2024 14:00:35 -0400 Subject: [PATCH 4/7] Add references to PlanContext and ExprContext --- datafusion/common/src/tree_node.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/datafusion/common/src/tree_node.rs b/datafusion/common/src/tree_node.rs index b3c3b02641dc6..98be3e8b1e462 100644 --- a/datafusion/common/src/tree_node.rs +++ b/datafusion/common/src/tree_node.rs @@ -47,8 +47,9 @@ macro_rules! handle_transform_recursion_up { /// 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`]) -/// and expression trees ([`PhysicalExpr`], [`Expr`]). +/// This trait is implemented for plans ([`ExecutionPlan`], [`LogicalPlan`]) and +/// expression trees ([`PhysicalExpr`], [`Expr`]) as well as Plan+Payload +/// combinations [`PlanContext`] and [`ExprContext`]. /// /// # Overview /// There are three categories of TreeNode APIs: @@ -99,6 +100,8 @@ macro_rules! handle_transform_recursion_up { /// [`PhysicalExpr`]: https://docs.rs/datafusion/latest/datafusion/physical_plan/trait.PhysicalExpr.html /// [`LogicalPlan`]: https://docs.rs/datafusion-expr/latest/datafusion_expr/logical_plan/enum.LogicalPlan.html /// [`Expr`]: https://docs.rs/datafusion-expr/latest/datafusion_expr/expr/enum.Expr.html +/// [`PlanContext`]: https://docs.rs/datafusion/latest/datafusion/physical_plan/tree_node/struct.PlanContext.html +/// [`ExprContext`]: https://docs.rs/datafusion/latest/datafusion/physical_expr/tree_node/struct.ExprContext.html pub trait TreeNode: Sized { /// Visit the tree node with a [`TreeNodeVisitor`], performing a /// depth-first walk of the node and its children. From 83440c540755d26ef861d9b15f797bd88b03d0a0 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 17 Apr 2024 14:02:06 -0400 Subject: [PATCH 5/7] refine TreeNodeRewriter docs --- datafusion/common/src/tree_node.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/datafusion/common/src/tree_node.rs b/datafusion/common/src/tree_node.rs index 98be3e8b1e462..28b91db74de18 100644 --- a/datafusion/common/src/tree_node.rs +++ b/datafusion/common/src/tree_node.rs @@ -152,8 +152,9 @@ pub trait TreeNode: Sized { /// children are visited), [`TreeNodeRewriter::f_up()`] is called in /// bottom-up order (after children are visited). /// - /// Note: If using the default [`TreeNodeRewriter::f_up`] that does nothing, - /// consider using [`Self::transform_down`]. + /// Note: If using the default [`TreeNodeRewriter::f_up`] or + /// [`TreeNodeRewriter::f_down`] that do nothing, consider using + /// [`Self::transform_down`] instead. /// /// # Return Value /// The returns value specifies how the tree walk should proceed. See From 9f0eead9593f820c07d0132f2cc36c718414f3da Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 17 Apr 2024 14:03:27 -0400 Subject: [PATCH 6/7] Add note about exists --- datafusion/common/src/tree_node.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/datafusion/common/src/tree_node.rs b/datafusion/common/src/tree_node.rs index 28b91db74de18..6d03a6f2733b5 100644 --- a/datafusion/common/src/tree_node.rs +++ b/datafusion/common/src/tree_node.rs @@ -55,7 +55,7 @@ macro_rules! handle_transform_recursion_up { /// There are three categories of TreeNode APIs: /// /// 1. "Inspecting" APIs to traverse a tree of `&TreeNodes`: -/// [`apply`] and [`visit`]. +/// [`apply`], [`visit`], [`exists`]. /// /// 2. "Transforming" APIs that traverse and consume a tree of `TreeNode`s /// producing possibly changed `TreeNode`s: [`transform`], [`transform_up`], @@ -66,7 +66,7 @@ macro_rules! handle_transform_recursion_up { /// /// | Traversal Order | Inspecting | Transforming | /// | --- | --- | --- | -/// | top-down | [`apply`] | [`transform_down`]| +/// | top-down | [`apply`], [`exists`] | [`transform_down`]| /// | bottom-up | | [`transform`] , [`transform_up`]| /// | combined with separate `f_down` and `f_up` closures | | [`transform_down_up`] | /// | combined with `f_down()` and `f_up()` in an object | [`visit`] | [`rewrite`] | @@ -77,6 +77,7 @@ macro_rules! handle_transform_recursion_up { /// /// [`apply`]: Self::apply /// [`visit`]: Self::visit +/// [`exists`]: Self::exists /// [`transform`]: Self::transform /// [`transform_up`]: Self::transform_up /// [`transform_down`]: Self::transform_down From 42ff89ed5fb44b372a6ec6a81beb186ba7a6e0b3 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 22 Apr 2024 07:31:43 -0400 Subject: [PATCH 7/7] Update datafusion/common/src/tree_node.rs Co-authored-by: Jeffrey Vo --- datafusion/common/src/tree_node.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/datafusion/common/src/tree_node.rs b/datafusion/common/src/tree_node.rs index 6d03a6f2733b5..af84abea6d77a 100644 --- a/datafusion/common/src/tree_node.rs +++ b/datafusion/common/src/tree_node.rs @@ -112,8 +112,7 @@ pub trait TreeNode: Sized { /// bottom-up order (after children are visited). /// /// # Return Value - /// The returns value specifies how the tree walk ended. - /// [`TreeNodeRecursion`] for details. + /// Specifies how the tree walk ended. See [`TreeNodeRecursion`] for details. /// /// # See Also: /// * [`Self::apply`] for inspecting nodes with a closure