egalpin commented on code in PR #10410:
URL: https://github.com/apache/pinot/pull/10410#discussion_r1134515017


##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java:
##########
@@ -179,97 +178,91 @@ public static Object getConvertedFinalResult(DataTable 
dataTable, ColumnDataType
   }
 
   /**
-   * Build a filter operator from the given FilterContext.
-   *
-   * It returns the FilterPlanNode to allow reusing plan level components such 
as predicate
-   * evaluator map
+   * Build pairs of filtered aggregation functions and corresponding transform 
operator.
    */
-  public static Pair<FilterPlanNode, BaseFilterOperator> 
buildFilterOperator(IndexSegment indexSegment,
-      QueryContext queryContext, FilterContext filterContext) {
-    FilterPlanNode filterPlanNode = new FilterPlanNode(indexSegment, 
queryContext, filterContext);
-    return Pair.of(filterPlanNode, filterPlanNode.run());
-  }
+  public static List<Pair<AggregationFunction[], TransformOperator>> 
buildFilteredAggregateTransformOperators(

Review Comment:
   Ya agreed, the resulting methods would not likely be reusable.  I do wonder 
if having multiple separate methods might help with isolating the intent of 
each block to aid readability, but I think "readability" is highly subjective.  
In any event, I don't think this is new with this PR, and could be altered as a 
separate, intentional follow up on a need basis. 



-- 
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