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



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java
##########
@@ -62,57 +69,25 @@ public AggregationPlanNode(IndexSegment indexSegment, 
QueryContext queryContext)
   public Operator<IntermediateResultsBlock> run() {
     assert _queryContext.getAggregationFunctions() != null;
 
-    int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
-    AggregationFunction[] aggregationFunctions = 
_queryContext.getAggregationFunctions();
+    boolean hasFilteredPredicates = _queryContext.isHasFilteredAggregations();
 
-    FilterPlanNode filterPlanNode = new FilterPlanNode(_indexSegment, 
_queryContext);
-    BaseFilterOperator filterOperator = filterPlanNode.run();
+    Pair<FilterPlanNode, BaseFilterOperator> filterOperatorPair =

Review comment:
       I am not sure if I understand -- why should star-tree be not used for a 
filtered aggregation? If a swim lane is able to use star tree to process the 
underlying predicate, shouldn't we allow that? Or is there a technical 
limitation to that?
   
   Regarding the forking, I agree the code readability was an issue. I have 
fixed the same




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to