siddharthteotia commented on code in PR #10636: URL: https://github.com/apache/pinot/pull/10636#discussion_r1186942587
########## pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java: ########## @@ -325,6 +325,18 @@ public static AggregationFunction getAggregationFunction(FunctionContext functio return new FourthMomentAggregationFunction(firstArgument, FourthMomentAggregationFunction.Type.KURTOSIS); case FOURTHMOMENT: return new FourthMomentAggregationFunction(firstArgument, FourthMomentAggregationFunction.Type.MOMENT); + case PINOTPARENTAGGREGATIONARGMAX: + return new ParentArgMinMaxAggregationFunction(arguments, true); + case PINOTPARENTAGGREGATIONARGMIN: + return new ParentArgMinMaxAggregationFunction(arguments, false); + case PINOTCHILDAGGREGATIONARGMAX: + return new ChildArgMinMaxAggregationFunction(arguments, true); + case PINOTCHILDAGGREGATIONARGMIN: Review Comment: I vaguely remember but we hit this in some recent work as part of multi stage as well. Aggregation functions that are not going to be used by the user in SQL also need to be exposed here and ideally they shouldn't. I think it happened for the 3rd / 4th moment / reduce functions Is it possible to only add `ARG_MIN` and `ARG_MAX` (the user level AggregationFunctions) in this interface ? -- 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