[GitHub] [pinot] walterddr commented on a diff in pull request #9570: [multistage] fix GROUP BY with no aggregate functions

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

walterddr commented on code in PR #9570:
URL: https://github.com/apache/pinot/pull/9570#discussion_r992827945


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/utils/ServerRequestUtils.java:
##########
@@ -182,12 +186,15 @@ private static void walkStageTree(StageNode node, PinotQuery pinotQuery, TableTy
       pinotQuery.setSelectList(CalciteRexExpressionParser.overwriteSelectList(
           ((ProjectNode) node).getProjects(), pinotQuery));
     } else if (node instanceof AggregateNode) {
-      // set agg list
-      pinotQuery.setSelectList(CalciteRexExpressionParser.addSelectList(pinotQuery.getSelectList(),
-          ((AggregateNode) node).getAggCalls(), pinotQuery));
       // set group-by list
       pinotQuery.setGroupByList(CalciteRexExpressionParser.convertGroupByList(
           ((AggregateNode) node).getGroupSet(), pinotQuery));
+      // set agg list - the elements that are selected for an aggregate query
+      // should include all of the group by list followed by the aggregate functions
+      // if other columns are selected upstream they will be projected out in a
+      // dedicated ProjectNode
+      pinotQuery.setSelectList(CalciteRexExpressionParser.addSelectList(pinotQuery.getGroupByList(),
+          ((AggregateNode) node).getAggCalls(), pinotQuery));

Review Comment:
   this is the main issue yes? 



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

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


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