[GitHub] [druid] clintropolis commented on a change in pull request #11208: fix count and average SQL aggregators on constant virtual columns

[GitBox <gi...@apache.org>]

clintropolis commented on a change in pull request #11208:
URL: https://github.com/apache/druid/pull/11208#discussion_r627869215



##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/CountSqlAggregator.java
##########
@@ -134,15 +134,17 @@ public Aggregation toDruidAggregation(
     } else {
       // Not COUNT(*), not distinct
       // COUNT(x) should count all non-null values of x.
-      return Aggregation.create(createCountAggregatorFactory(
+      AggregatorFactory theCount = createCountAggregatorFactory(

Review comment:
       Instead I have just removed the `VirtualColumn` tracking from `Aggregation` entirely in favor of replacing it with a new `getRequiredColumns ` that can list all columns between agg factories and the post agg, and using the new find method on the `VirtualColumnRegistry` to look them up.




-- 
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:
users@infra.apache.org



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