ozankabak commented on code in PR #8672:
URL: https://github.com/apache/arrow-datafusion/pull/8672#discussion_r1438396179
##########
datafusion/expr/src/tree_node/expr.rs:
##########
@@ -24,16 +24,13 @@ use crate::expr::{
};
use crate::{Expr, GetFieldAccess};
-use datafusion_common::tree_node::{TreeNode, VisitRecursion};
+use datafusion_common::tree_node::TreeNode;
use datafusion_common::{internal_err, DataFusionError, Result};
impl TreeNode for Expr {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- let children = match self {
- Expr::Alias(Alias{expr,..})
+ fn children_nodes(&self) -> Vec<Self> {
Review Comment:
Sometimes there may be cases where one needs to do multiple passes over the
same tree (e.g. doing one top down and one bottom up pass consecutively). Or we
may want to run different logic on the same tree.
So it may be a little premature to conclude we can do away with "derived"
trees and just keep the base four.
I suggest we take small steps and progressively discover the
requirements/use cases. I think refactoring to get rid of duplicate
apply_children and map_children is a good first step.
--
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]
