peter-toth commented on code in PR #13467:
URL: https://github.com/apache/datafusion/pull/13467#discussion_r1848074244
##########
datafusion/expr/src/logical_plan/tree_node.rs:
##########
@@ -81,7 +79,7 @@ impl TreeNode for LogicalPlan {
expr,
input,
schema,
- }) => rewrite_arc(input, f)?.update_data(|input| {
+ }) => input.map_elements(f)?.update_data(|input| {
Review Comment:
> IIUC map_children defines how LogicalPlan's construct a tree topology,
while map_elements is resolving the link between nodes.
Yes, the container trait supposes that the elements in it are sibling nodes.
Basically this is why we can use it when we implement `apply_children()` /
`map_children()`. The blanket implementations for the trait makes it very
simple to use with `Expr` and `LogicalPlan` as those logical plan treenodes
define their children as a composition of `Vec`s, `Option`s, tuples, `Box`es
and `Arc`s.
Here in `LogicalPlan` a treenode has mostly only 1 child (except for `Join`
and `Union` maybe) so input (`Arc<LogicalPlan>`) is just a single element
container so this PR doesn't have much benefit, but I think it is worth to
allign `Expr` and `LogicalPlan` code.
`TreeNode::map_children` and `TreeNodeContainer:: map_elements` are similar
in a way that both apply `f` to their children / elements of a `TreeNode` /
`TreeNodeContainer`. But we use `TreeNode` to define different kids of logical
and physical plan trees, while `TreeNodeContainer` can be used to simplify
implementation of `apply_children` / `map_children()` as children are siblings.
--
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
