[GitHub] [pinot] atris commented on a change in pull request #7916: FILTER Clauses for Aggregates

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

atris commented on a change in pull request #7916:
URL: https://github.com/apache/pinot/pull/7916#discussion_r785767852



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
##########
@@ -90,9 +92,11 @@
 
   // Pre-calculate the aggregation functions and columns for the query so that it can be shared across all the segments
   private AggregationFunction[] _aggregationFunctions;
-  private List<Pair<AggregationFunction, FilterContext>> _filteredAggregationFunctions;
+

Review comment:
       FilterableAggregationFunction provides a nice abstraction for defining the sub aggregation, filter and associated context (for collecting same predicate aggregation functions together) without requiring a lot of code ripping in QueryContext. It also helps maintain the order aggregations as present in the original SELECT list.
   
   FilterableAggregationFunction is limited to the execution of filtered aggregates and is not evident anywhere else. I would advocate keeping this class for simplicity.




-- 
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-unsubscribe@pinot.apache.org

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



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