xiangfu0 commented on code in PR #18817:
URL: https://github.com/apache/pinot/pull/18817#discussion_r3446248252
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/logical/PinotLogicalAggregate.java:
##########
@@ -92,6 +97,35 @@ public PinotLogicalAggregate copy(RelTraitSet traitSet,
RelNode input, Immutable
_leafReturnFinalResult, _collations, _limit);
}
+ /// Whether this LEAF aggregate must synthesize the {@code $groupingId}
discriminator column. Only a LEAF
+ /// grouping-set aggregate does: it is converted to a single-stage query
that expands each row across the
+ /// grouping sets and appends {@code $groupingId}; the multi-stage final
stage then groups on it.
+ public boolean emitsGroupingId() {
+ return _aggType == AggType.LEAF && getGroupType() != Group.SIMPLE;
+ }
+
+ @Override
+ protected RelDataType deriveRowType() {
+ RelDataType rowType = super.deriveRowType();
+ if (!emitsGroupingId()) {
+ return rowType;
+ }
+ // Insert the synthetic $groupingId INT column right after the union
group-by columns (before the aggregate
+ // columns), matching the single-stage leaf output layout: [group keys...,
$groupingId, aggregates...].
+ RelDataTypeFactory typeFactory = getCluster().getTypeFactory();
+ RelDataTypeFactory.Builder builder = typeFactory.builder();
+ List<RelDataTypeField> fields = rowType.getFieldList();
+ int groupCount = getGroupCount();
+ for (int i = 0; i < groupCount; i++) {
+ builder.add(fields.get(i));
+ }
+ builder.add(GroupingSets.GROUPING_ID_COLUMN,
typeFactory.createSqlType(SqlTypeName.INTEGER));
Review Comment:
This changes the leaf-stage wire row layout, but there is no mixed-version
gate for the new broker/server contract. Older servers will ignore
`AggregateNode.groupingSets`, build a plain grouped leaf `PinotQuery`, and
return the pre-`$groupingId` shape during a rolling upgrade. That breaks
Pinot's mixed-version broker/server guarantee for this feature. Can we gate
this plan shape on worker capability, or fall back to single-stage / explicit
failure until all servers are upgraded?
--
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]
