[GitHub] [doris] morrySnow commented on a diff in pull request #15322: [fix](nereids) Fix bugs of group by constants

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

morrySnow commented on code in PR #15322:
URL: https://github.com/apache/doris/pull/15322#discussion_r1057061091


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/EliminateGroupByConstant.java:
##########
@@ -50,12 +50,18 @@ public Rule build() {
             List<Expression> groupByExprs = aggregate.getGroupByExpressions();
             List<NamedExpression> outputExprs = aggregate.getOutputExpressions();
             Set<Expression> slotGroupByExprs = Sets.newLinkedHashSet();
+            Expression lit = null;
             for (Expression expression : groupByExprs) {
                 expression = FoldConstantRule.INSTANCE.rewrite(expression);
                 if (!(expression instanceof Literal)) {
                     slotGroupByExprs.add(expression);
+                } else {
+                    lit = expression;
                 }
             }
+            if (slotGroupByExprs.isEmpty() && lit != null) {
+                slotGroupByExprs.add(lit);
+            }

Review Comment:
   why we must keep at least one group expression?
   if all group by expressions are constant, it is just like aggregate without group by keys. So in theory, we do not need to keep any constant group by key



-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org