FrankChen021 commented on code in PR #20452:
URL: https://github.com/apache/druid/pull/20452#discussion_r4155524383
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/ProjectionSpecTranslator.java:
##########
@@ -501,42 +679,35 @@ private static AggregateProjectionSpec lift(
}
final Query<?> query = druidQuery.getQuery();
- final List<DimensionSpec> dimensions;
- final List<AggregatorFactory> aggregators;
- final VirtualColumnsAndFilter virtualColumnsAndFilter;
if (query instanceof GroupByQuery) {
final GroupByQuery groupBy = (GroupByQuery) query;
- dimensions = groupBy.getDimensions();
- aggregators = groupBy.getAggregatorSpecs();
- virtualColumnsAndFilter = new VirtualColumnsAndFilter(
- groupBy.getVirtualColumns(),
- groupBy.getDimFilter(),
- groupBy.getIntervals()
+ return new PlannedAggregation(
Review Comment:
[P1] Reject grouping sets in rollup base definitions
**Finding:** When a __base body uses GROUPING SETS, the projection engine
permits that syntax and the planned GroupByQuery retains subtotal groups, but
plannedAggregation copies only dimensions and aggregators. liftRollupBaseTable
then accepts the definition as an ordinary rollup spec, so ingestion groups
every row by the full dimension tuple instead of materializing the subtotal
rows described by the DDL body.
**Suggestion:** Reject nontrivial grouping subtotals on the rollup-base
path, or preserve them in a representation the base-table spec can execute, and
add a regression test for GROUPING SETS.
--
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]
