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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
