suneet-s commented on a change in pull request #9122: Add SQL GROUPING SETS
support.
URL: https://github.com/apache/druid/pull/9122#discussion_r383441586
##########
File path:
processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java
##########
@@ -369,13 +374,13 @@ public boolean doMergeResults(final GroupByQuery query)
.map(AggregatorFactory::getCombiningFactory)
.collect(Collectors.toList())
)
- .withSubtotalsSpec(null)
- .withDimFilter(null);
-
+ .withVirtualColumns(VirtualColumns.EMPTY)
+ .withDimFilter(null)
+ .withSubtotalsSpec(null);
Review comment:
not really part of this change - maybe worth filing an issue for. The
`with...` interface returns a query object, so chaining doesn't really make
sense here. Maybe we should consider making that return the `Builder` so
callers can call `.build()` when they're done setting all the fields. In this
example it looks like it creates 4 intermediate GroupByQyery objects and a
Builder for each object
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
