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

Reply via email to