xiangfu0 commented on code in PR #18662:
URL: https://github.com/apache/pinot/pull/18662#discussion_r3352014268
##########
pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java:
##########
@@ -163,7 +170,7 @@ private OrderByValueExtractor
getOrderByValueExtractor(ExpressionContext express
int index = _filteredAggregationIndexMap.get(Pair.of(aggregation, filter));
// For final aggregate result, we can handle it the same way as group key
- return _hasFinalInput ? new
GroupByExpressionExtractor(_numGroupByExpressions + index)
+ return _hasFinalInput ? new GroupByExpressionExtractor(_numKeyColumns +
index)
Review Comment:
Fixed in 4296dd7. Added a dedicated `GroupingExtractor` in
`TableResizer.getOrderByValueExtractor(...)` that mirrors
`PostAggregationHandler`: it reads the synthetic `$grouping_id` discriminator
column and computes the bitmask over the argument columns (first arg = MSB),
instead of falling through to `PostAggregationFunctionExtractor`. This covers
both the IndexedTable ORDER BY path (combine + reduce) and the per-set
segment-trim comparator.
Added a regression test
`GroupingSetsQueriesTest#testOrderByGroupingFunction` covering `... GROUP BY
ROLLUP(d1, d2) ORDER BY GROUPING(d2)` and `... ORDER BY GROUPING_ID(d1, d2)`
end to end (and hardened the test helper to surface broker exceptions).
Subtlety worth noting: these functions canonicalize to `grouping` /
`groupingid` (underscores stripped), so the extractor matches `groupingid`,
consistent with `PostAggregationHandler`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
