[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3972: draft: refactor optimizer to avoid every rule must recursive children.

[GitBox <gi...@apache.org>]

alamb commented on code in PR #3972:
URL: https://github.com/apache/arrow-datafusion/pull/3972#discussion_r1008685580


##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -342,6 +342,53 @@ impl LogicalPlan {
         self.accept(&mut visitor)?;
         Ok(visitor.using_columns)
     }
+
+    pub fn clone_with_inputs(&self, inputs: Vec<LogicalPlan>) -> Result<LogicalPlan, DataFusionError> {

Review Comment:
   This has a non trivial amount in common with `from_plan`: 
   https://github.com/apache/arrow-datafusion/blob/4cb8ac094ee88dc0023b4cde1b39840a0a362ee0/datafusion/expr/src/utils.rs#L363
   
   
   I wonder if we could make `from_plan` easier to find / use / more general 🤔 



##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -254,6 +261,45 @@ impl Optimizer {
         debug!("Optimizer took {} ms", start_time.elapsed().as_millis());
         Ok(new_plan)
     }
+
+    fn optimize_node(&self, rule: &Arc<dyn OptimizerRule + Send + Sync>, plan: &LogicalPlan, optimizer_config: &mut OptimizerConfig) -> Result<LogicalPlan> {
+        /// We can do Batch optimize
+        /// for rule in self.rules {
+        ///     let result = rule.optimize(&plan, optimizer_config);
+        ///         plan = result?;
+        ///         self.stats.count_rule(rule);
+        ///     }
+        /// }
+        let result = rule.optimize(&plan, optimizer_config);
+        result
+    }
+
+    fn optimize_inputs(&self, rule: &Arc<dyn OptimizerRule + Send + Sync>, plan: &LogicalPlan, optimizer_config: &mut OptimizerConfig) -> Result<LogicalPlan> {
+        let result: Result<Vec<LogicalPlan>> = plan
+            .inputs()
+            .into_iter()
+            .map(|sub_plan| self.optimize_recursively(rule, sub_plan, optimizer_config))
+            .collect();
+        let inputs = result?;
+        plan.clone_with_inputs(inputs)
+    }
+

Review Comment:
   I like where this is headed @jackwener  👍 
   
   



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org