xuzifu666 commented on code in PR #4983:
URL: https://github.com/apache/calcite/pull/4983#discussion_r3360126616
##########
core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java:
##########
@@ -864,6 +920,23 @@ private static int remap(ImmutableBitSet groupSet, int
arg) {
return arg < 0 ? -1 : groupSet.indexOf(arg);
}
+ private static RelCollation remapCollationForGroupingSets(RelCollation
collation,
+ ImmutableBitSet fullGroupSet) {
+ if (collation.equals(RelCollations.EMPTY)) {
+ return RelCollations.EMPTY;
+ }
+ // Remap each field index through the fullGroupSet
+ final List<RelFieldCollation> remappedFCs = new ArrayList<>();
+ for (RelFieldCollation fc : collation.getFieldCollations()) {
+ int originalIdx = fc.getFieldIndex();
+ int newIdx = fullGroupSet.indexOf(originalIdx);
+ if (newIdx >= 0) {
+ remappedFCs.add(fc.withFieldIndex(newIdx));
Review Comment:
In my view it's acceptable for result collation to have fewer elements
because:
1. fullGroupSet only contains columns that are part of some grouping set. If
an ORDER BY column is not in fullGroupSet, it means that column is not in any
grouping set.
2. Columns not in any grouping set have inconsistent values within each
group — different rows with the same group key may have different values for
that column.
3. Sorting by columns with inconsistent values is meaningless. It's safe to
silently drop them from the collation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
