[GitHub] [pinot] egalpin commented on a diff in pull request #10410: Simplify filtered aggregate transform operator creation

["egalpin (via GitHub)" <gi...@apache.org>]

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


##########
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:
   This method was already huge, so it's a not exactly a new "issue" introduced by this PR, but what are your thoughts on breaking this method into a few logical methods?  Ex. the loop to create the hashmap of `Map<FilterContext, Pair<BaseFilterOperator, List<AggregationFunction>>>` (line 200) could be its own method?  Likewise `// Create the transform operators` on line 230-256?



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