alamb commented on code in PR #8664:
URL: https://github.com/apache/arrow-datafusion/pull/8664#discussion_r1438883403
##########
datafusion/common/src/tree_node.rs:
##########
@@ -136,6 +172,23 @@ pub trait TreeNode: Sized {
after_op.map_children(|node| node.transform_down_mut(op))
}
+ /// Transforms the tree using `f` pre-preorder (top-down) traversal. The
`f_down`
+ /// closure takes payloads that it propagates down during the
transformation.
+ ///
+ /// The `f_down` closure takes `FD` type payload from its parent and
returns `Vec<FD>`
+ /// type payload to propagate down to its children. One `FD` element is
propagated
+ /// down to each child.
+ fn transform_down_with_payload<F, P>(self, f: &mut F, payload: P) ->
Result<Self>
+ where
+ F: FnMut(Self, P) -> Result<(Transformed<Self>, Vec<P>)>,
+ {
+ let (new_node, new_payload) = f(self, payload)?;
Review Comment:
We could potentially reduce redundancy by implementing this function terms
of `transform_with_payload` and empty `f_up` and `PU=()` -- it probably
doesn't matter but I figured I would bring it up
##########
datafusion/core/src/physical_optimizer/enforce_distribution.rs:
##########
@@ -269,11 +266,18 @@ impl PhysicalOptimizerRule for EnforceDistribution {
/// 4) If the current plan is Projection, transform the requirements to the
columns before the Projection and push down requirements
/// 5) For other types of operators, by default, pushdown the parent
requirements to children.
///
+type RequiredKeyOrdering = Vec<Arc<dyn PhysicalExpr>>;
Review Comment:
It might make this code simpler to make this `struct RequiredKeyOrder` and
give its methods names rather than dealing with `Vec<Vec<...>>` -- but that can
be a refactor for a different PR perhaps
--
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]
