peter-toth commented on code in PR #10473:
URL: https://github.com/apache/datafusion/pull/10473#discussion_r1650116199
##########
datafusion/optimizer/src/common_subexpr_eliminate.rs:
##########
@@ -351,56 +468,108 @@ impl CommonSubexprEliminate {
schema: orig_schema,
..
} = aggregate;
- let mut expr_stats = ExprStats::new();
-
// track transformed information
let mut transformed = false;
- // rewrite inputs
- let group_arrays = to_arrays(&group_expr, &mut expr_stats,
ExprMask::Normal)?;
- let aggr_arrays = to_arrays(&aggr_expr, &mut expr_stats,
ExprMask::Normal)?;
-
let name_perserver = NamePreserver::new_for_projection();
let saved_names = aggr_expr
.iter()
.map(|expr| name_perserver.save(expr))
.collect::<Result<Vec<_>>>()?;
- // rewrite both group exprs and aggr_expr
- let rewritten = self.rewrite_expr(
- vec![group_expr, aggr_expr],
- &[&group_arrays, &aggr_arrays],
- unwrap_arc(input),
- &expr_stats,
- config,
- )?;
- transformed |= rewritten.transformed;
- let (mut new_expr, new_input) = rewritten.data;
-
- // note the reversed pop order.
- let new_aggr_expr = pop_expr(&mut new_expr)?;
- let new_group_expr = pop_expr(&mut new_expr)?;
+ let mut expr_stats = ExprStats::new();
+ // rewrite inputs
+ let (group_found_common, group_arrays) =
+ self.to_arrays(&group_expr, &mut expr_stats, ExprMask::Normal)?;
+ let (aggr_found_common, aggr_arrays) =
+ self.to_arrays(&aggr_expr, &mut expr_stats, ExprMask::Normal)?;
+ let (new_aggr_expr, new_group_expr, new_input) =
+ if group_found_common || aggr_found_common {
+ // rewrite both group exprs and aggr_expr
+ let rewritten = self.rewrite_expr(
+ vec![group_expr.clone(), aggr_expr.clone()],
+ vec![group_arrays, aggr_arrays],
+ unwrap_arc(input),
+ &expr_stats,
+ config,
+ )?;
+ assert!(rewritten.transformed);
+ transformed |= rewritten.transformed;
+ let (mut new_expr, new_input) = rewritten.data;
+
+ // note the reversed pop order.
+ let new_aggr_expr = pop_expr(&mut new_expr)?;
+ let new_group_expr = pop_expr(&mut new_expr)?;
+
+ (new_aggr_expr, new_group_expr, Arc::new(new_input))
+ } else {
+ (aggr_expr, group_expr, input)
+ };
// create potential projection on top
let mut expr_stats = ExprStats::new();
- let new_input_schema = Arc::clone(new_input.schema());
- let aggr_arrays = to_arrays(
+ let (aggr_found_common, aggr_arrays) = self.to_arrays(
&new_aggr_expr,
&mut expr_stats,
ExprMask::NormalAndAggregates,
)?;
- let mut common_exprs = IndexMap::new();
- let mut rewritten_exprs = self.rewrite_exprs_list(
- vec![new_aggr_expr.clone()],
- &[&aggr_arrays],
- &expr_stats,
- &mut common_exprs,
- &config.alias_generator(),
- )?;
- transformed |= rewritten_exprs.transformed;
- let rewritten = pop_expr(&mut rewritten_exprs.data)?;
+ if aggr_found_common {
+ let mut common_exprs = CommonExprs::new();
+ let mut rewritten_exprs = self.rewrite_exprs_list(
+ vec![new_aggr_expr.clone()],
Review Comment:
Added in
https://github.com/apache/datafusion/pull/10473/commits/ee61224f1a948c38ac99ae53d9d426bb69da9548.
--
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
