ozankabak commented on code in PR #10913:
URL: https://github.com/apache/datafusion/pull/10913#discussion_r1641427524


##########
datafusion/core/src/physical_optimizer/combine_partial_final_agg.rs:
##########
@@ -144,8 +144,12 @@ fn can_combine(final_agg: GroupExprsRef, partial_agg: 
GroupExprsRef) -> bool {
     let (input_group_by, input_aggr_expr, input_filter_expr) =
         normalize_group_exprs(partial_agg);
 
-    final_group_by.eq(&input_group_by)

Review Comment:
   We were trying to think whether it is possible for a valid plan to have a 
consecutive Partial/Final duo with differing GROUP BY expressions (unless it is 
manually generated that way w/o a query).
   
   We weren't able to find an example of this and started to think it is not 
possible. That's why that check was deemed to inhibit better plans in some 
cases without adding any real protection.
   
   We would appreciate some brain cycles from the community on this. If our 
suspicion is correct, this small PR will give us better plans in many cases.



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