mihaibudiu commented on code in PR #3703:
URL: https://github.com/apache/calcite/pull/3703#discussion_r1501115489


##########
core/src/main/java/org/apache/calcite/tools/RelBuilder.java:
##########
@@ -2510,7 +2510,7 @@ private RelBuilder aggregate_(GroupKeyImpl groupKey,
     // duplicates, then add a Project.
     final Set<AggregateCall> callSet = new HashSet<>();
     final PairList<Integer, @Nullable String> projects = PairList.of();
-    Util.range(groupSet.cardinality())
+    Util.range(groupSet2.cardinality())

Review Comment:
   Sure, but this means that #3700 is actually equivalent to this fix.
   So that PR probably fixes both issues as well.
   Is there a more descriptive name that could be used instead of groupSet2? 
Maybe that's the root problem: it is very hard to tell what groupSet2 means. 
But if it was called e.g., replacementGroupSet, then it would become apparent 
where it is misused.



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

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

Reply via email to