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: [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]
