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