peter-toth commented on code in PR #11683:
URL: https://github.com/apache/datafusion/pull/11683#discussion_r1709805716


##########
datafusion/optimizer/src/common_subexpr_eliminate.rs:
##########
@@ -227,69 +227,65 @@ impl CommonSubexprEliminate {
                     .into_iter()
                     .zip(arrays.iter())
                     .map(|(expr, id_array)| {
-                        let replaced = replace_common_expr(
+                        replace_common_expr(
                             expr,
                             id_array,
                             expr_stats,
                             common_exprs,
                             alias_generator,
-                        )?;
-                        // remember if this expression was actually replaced
-                        transformed |= replaced.transformed;
-                        Ok(replaced.data)
+                        )
                     })
                     .collect::<Result<Vec<_>>>()
             })
             .collect::<Result<Vec<_>>>()
-            .map(|rewritten_exprs_list| {
-                // propagate back transformed information
-                Transformed::new_transformed(rewritten_exprs_list, transformed)
-            })
     }
 
-    /// Rewrites the expression in `exprs_list` with common sub-expressions
-    /// replaced with a new column and adds a ProjectionExec on top of `input`
-    /// which computes any replaced common sub-expressions.
+    /// Extracts common sub-expressions and rewrites `exprs_list`.
     ///
     /// Returns a tuple of:
     /// 1. The rewritten expressions
-    /// 2. A `LogicalPlan::Projection` with input of `input` that computes any
-    ///    common sub-expressions that were used
-    fn rewrite_expr(
+    /// 2. An optional tuple that contains the extracted common 
sub-expressions and the
+    ///    original `exprs_list`.
+    fn find_common_exprs(
         &self,
         exprs_list: Vec<Vec<Expr>>,
-        arrays_list: Vec<Vec<IdArray>>,
-        input: LogicalPlan,
-        expr_stats: &ExprStats,
         config: &dyn OptimizerConfig,
-    ) -> Result<Transformed<(Vec<Vec<Expr>>, LogicalPlan)>> {
-        let mut transformed = false;
-        let mut common_exprs = CommonExprs::new();
-
-        let rewrite_exprs = self.rewrite_exprs_list(
-            exprs_list,
-            arrays_list,
-            expr_stats,
-            &mut common_exprs,
-            &config.alias_generator(),
-        )?;
-        transformed |= rewrite_exprs.transformed;
+        expr_mask: ExprMask,
+    ) -> Result<Transformed<(Vec<Vec<Expr>>, FindCommonExprResult)>> {

Review Comment:
   Thanks for PR, I've just merged it.



-- 
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

Reply via email to