xiangfu0 commented on code in PR #18662:
URL: https://github.com/apache/pinot/pull/18662#discussion_r3385870001
##########
pinot-core/src/main/java/org/apache/pinot/core/query/request/context/utils/QueryContextConverterUtils.java:
##########
@@ -114,6 +114,26 @@ public static QueryContext getQueryContext(PinotQuery
pinotQuery) {
}
}
+ /// GROUP BY GROUPING SETS / ROLLUP / CUBE: the wire carries one
membership bitmask per set over
+ /// groupByExpressions (the union of all grouping columns). Decode each
mask into the sorted list of
+ /// participating column indexes; a mask of 0 yields the empty
(grand-total) set (). Null when this is a
+ /// plain GROUP BY query.
+ List<int[]> groupingSets = null;
+ List<Integer> groupingSetMasks = pinotQuery.getGroupingSetMasks();
+ if (groupingSetMasks != null) {
+ groupingSets = new ArrayList<>(groupingSetMasks.size());
+ for (int mask : groupingSetMasks) {
+ int[] indexes = new int[Integer.bitCount(mask)];
+ int idx = 0;
+ for (int bit = 0; bit < Integer.SIZE && idx < indexes.length; bit++) {
+ if ((mask & (1 << bit)) != 0) {
+ indexes[idx++] = bit;
+ }
+ }
+ groupingSets.add(indexes);
+ }
+ }
Review Comment:
Fixed in b6a686065f — `QueryContextConverterUtils` now validates each mask
before decoding (must be non-negative with no bits >= the group-by union size,
covering the empty-union case) and throws a clear `IllegalStateException`
instead of risking out-of-bounds access in the group key generator. Regression
test:
`BrokerRequestToQueryContextConverterTest#testInvalidGroupingSetMasksRejected`.
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -690,6 +724,172 @@ private static List<Expression>
convertSelectList(SqlNodeList selectList) {
return selectExpr;
}
+ /// Converts the GROUP BY clause into {@link PinotQuery#groupByList} and
(when grouping constructs are
+ /// present) {@link PinotQuery#groupingSets}.
+ ///
+ /// For a plain GROUP BY (no ROLLUP / CUBE / GROUPING SETS) this behaves
exactly like
+ /// {@link #convertSelectList} and leaves {@code groupingSets} unset, so
non-grouping-set queries are
+ /// unchanged. When grouping constructs are present, the grouping elements
are cross-multiplied into the
+ /// canonical, de-duplicated list of grouping sets (standard SQL semantics,
e.g.
+ /// {@code GROUP BY a, ROLLUP(b, c)} produces {@code {a,b,c}, {a,b}, {a}});
the ordered, de-duplicated
+ /// union of all participating columns is stored as {@code groupByList}, and
each grouping set is stored
+ /// as the sorted list of indexes into that union (an empty list = the
grand-total set {@code ()}).
Review Comment:
Fixed in b6a686065f — the doc comment now references
`PinotQuery#groupingSetMasks` and describes the per-set membership bitmask
encoding (bit i set iff union column i participates; 0 = grand-total set).
--
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]
