[GitHub] [druid] kaijianding commented on a change in pull request #11379: improve groupBy query granularity translation with 2x query performance improve when issued from sql layer

[GitBox]

kaijianding commented on a change in pull request #11379:
URL: https://github.com/apache/druid/pull/11379#discussion_r661147244



##########
File path: 
processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java
##########
@@ -207,9 +210,17 @@ private GroupByQuery(
       Preconditions.checkArgument(spec != null, "dimensions has null 
DimensionSpec");
     }
 
+    List<String> dimensionNames = this.dimensions
+        .stream()
+        .map(DimensionSpec::getOutputName)
+        .collect(Collectors.toList());
+    String timestampField = getContextValue(CTX_TIMESTAMP_RESULT_FIELD);
+    if (timestampField != null) {
+      dimensionNames.add(timestampField);

Review comment:
       the timestampField maybe referenced by the postAgg, so the 
timestampField should be added back to the dependencies of the postAggs here, 
otherwise the constructor will fail.
   
   But I think postAggs should not be applied on compute nodes, so I will 
remove the postAggs in `GroupByStrategyV2.newQuery` and remove timestampField 
codes from `GroupByQuery`




-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org