mayankshriv commented on a change in pull request #5613:
URL: https://github.com/apache/incubator-pinot/pull/5613#discussion_r445968533



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java
##########
@@ -83,16 +73,69 @@
    *                    <li> Required: Last expression is the one that will be 
evaluated to compute final result. </li>
    *                    </ul>
    */
-  public DistinctCountThetaSketchAggregationFunction(List<String> arguments)
+  public DistinctCountThetaSketchAggregationFunction(List<ExpressionContext> 
arguments)
       throws SqlParseException {
-    int numExpressions = arguments.size();
+    int numArguments = arguments.size();
+
+    // NOTE: This function expects at least 3 arguments: theta-sketch column, 
parameters, post-aggregation expression.
+    Preconditions.checkArgument(numArguments >= 3,
+        "DistinctCountThetaSketch expects at least three arguments 
(theta-sketch column, parameters, post-aggregation expression), got: ",
+        numArguments);
+
+    // Initialize the theta-sketch column
+    _thetaSketchColumn = arguments.get(0);
+    Preconditions.checkArgument(_thetaSketchColumn.getType() == 
ExpressionContext.Type.IDENTIFIER,
+        "First argument of DistinctCountThetaSketch must be identifier 
(theta-sketch column)");
+
+    // Initialize the theta-sketch parameters
+    ExpressionContext paramsExpression = arguments.get(1);
+    Preconditions.checkArgument(paramsExpression.getType() == 
ExpressionContext.Type.LITERAL,
+        "Second argument of DistinctCountThetaSketch must be literal 
(parameters)");
+    _thetaSketchParams = 
ThetaSketchParams.fromString(paramsExpression.getLiteral());
 
-    // This function expects at least 3 arguments: Theta Sketch Column, 
Predicates & final aggregation expression.
-    Preconditions.checkArgument(numExpressions >= 3, "DistinctCountThetaSketch 
expects at least three arguments, got: ",
-        numExpressions);
+    // Initialize the theta-sketch set operation builder
+    _setOperationBuilder = getSetOperationBuilder();
 
-    // Initialize all the internal state.
-    init(arguments);
+    // Initialize the input expressions

Review comment:
       what's the issue with calling init()? 




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to