swaminathanmanish commented on code in PR #14373: URL: https://github.com/apache/pinot/pull/14373#discussion_r1855425265
########## pinot-core/src/main/java/org/apache/pinot/core/segment/processing/aggregator/DistinctCountCPCSketchAggregator.java: ########## @@ -30,7 +31,7 @@ public DistinctCountCPCSketchAggregator() { } @Override - public Object aggregate(Object value1, Object value2) { + public Object aggregate(Object value1, Object value2, Map<String, String> functionParameters) { Review Comment: @davecromberge - Not sure if it needs to be an Object. Instead of passing Map<String, String> to the interface, can we have the class for FunctionParams and individual params as typed fields in that class. We will extract out these fields in getAggregationFunctionParameters. If Im missing something, please let me know. If we are going to use these params conditionally with no defaults then we dont have a choice, other than to use Map. Class AggregateFunctionParams { int _sketchNominalEntries; // This will be default or custom value ...} AggregateFunctionParams will be created in getAggregationFunctionParameters. Will be have a Map<String, AggregateFunctionParams> Object aggregate(Object value1, Object value2, AggregateFunctionParams aggregateFunctionParams) { int sketchNominalEntries = aggregateFunctionParams._sketchNominalEntries; ... } -- 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