bkuang88 commented on a change in pull request #5832:
URL: https://github.com/apache/incubator-pinot/pull/5832#discussion_r468136912
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java
##########
@@ -576,6 +616,69 @@ private SetOperationBuilder getSetOperationBuilder() {
:
SetOperation.builder().setNominalEntries(_thetaSketchParams.getNominalEntries());
}
+ /**
+ * Validates that the function context's substitution parameters ($1, $2,
etc) does not exceed the number
+ * of predicates passed into the post-aggregation function.
+ *
Review comment:
Done
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java
##########
@@ -576,6 +616,69 @@ private SetOperationBuilder getSetOperationBuilder() {
:
SetOperation.builder().setNominalEntries(_thetaSketchParams.getNominalEntries());
}
+ /**
+ * Validates that the function context's substitution parameters ($1, $2,
etc) does not exceed the number
+ * of predicates passed into the post-aggregation function.
+ *
+ * For example, if the post aggregation function is:
+ * INTERSECT($1, $2, $3)
+ *
+ * But there are only 2 arguments passed into the aggregation function,
throw an error
+ * @param context The parsed function context that's a tree structure
+ * @param numPredicates Max number of predicates available to be substituted
+ */
+ private static void validatePostAggregationExpression(final
ExpressionContext context, final int numPredicates) {
+ if (context.getType() == ExpressionContext.Type.LITERAL) {
+ throw new IllegalArgumentException("Invalid post-aggregation function
expression syntax.");
+ }
+
+ if (context.getType() == ExpressionContext.Type.IDENTIFIER) {
+ int id = extractSubstitutionPosition(context.getIdentifier());
+ if (id <= 0)
+ throw new IllegalArgumentException("Argument substitution starts at
$1");
+ if (id > numPredicates)
+ throw new IllegalArgumentException("Argument substitution exceeded
number of predicates");
+ // if none of the invalid conditions are met above, exit out early
+ return;
+ }
+
+ if (!MergeFunction.isValid(context.getFunction().getFunctionName())) {
+ final String allowed =
+ Arrays.stream(MergeFunction.values())
+ .map(MergeFunction::name)
+ .collect(Collectors.joining(","));
+ throw new IllegalArgumentException(
+ String.format("Invalid Theta Sketch aggregation function. Allowed:
[%s]", allowed));
+ }
+
+ switch
(MergeFunction.valueOf(context.getFunction().getFunctionName().toUpperCase())) {
+ case SET_DIFF:
+ // set diff can only have 2 arguments
+ if (context.getFunction().getArguments().size() != 2) {
+ throw new IllegalArgumentException("SET_DIFF function can only have
2 arguments.");
+ }
+ break;
Review comment:
Done
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java
##########
@@ -576,6 +616,69 @@ private SetOperationBuilder getSetOperationBuilder() {
:
SetOperation.builder().setNominalEntries(_thetaSketchParams.getNominalEntries());
}
+ /**
+ * Validates that the function context's substitution parameters ($1, $2,
etc) does not exceed the number
+ * of predicates passed into the post-aggregation function.
+ *
+ * For example, if the post aggregation function is:
+ * INTERSECT($1, $2, $3)
+ *
+ * But there are only 2 arguments passed into the aggregation function,
throw an error
+ * @param context The parsed function context that's a tree structure
+ * @param numPredicates Max number of predicates available to be substituted
+ */
+ private static void validatePostAggregationExpression(final
ExpressionContext context, final int numPredicates) {
+ if (context.getType() == ExpressionContext.Type.LITERAL) {
+ throw new IllegalArgumentException("Invalid post-aggregation function
expression syntax.");
+ }
+
+ if (context.getType() == ExpressionContext.Type.IDENTIFIER) {
+ int id = extractSubstitutionPosition(context.getIdentifier());
+ if (id <= 0)
+ throw new IllegalArgumentException("Argument substitution starts at
$1");
+ if (id > numPredicates)
+ throw new IllegalArgumentException("Argument substitution exceeded
number of predicates");
+ // if none of the invalid conditions are met above, exit out early
+ return;
+ }
+
+ if (!MergeFunction.isValid(context.getFunction().getFunctionName())) {
+ final String allowed =
+ Arrays.stream(MergeFunction.values())
+ .map(MergeFunction::name)
+ .collect(Collectors.joining(","));
+ throw new IllegalArgumentException(
+ String.format("Invalid Theta Sketch aggregation function. Allowed:
[%s]", allowed));
+ }
+
+ switch
(MergeFunction.valueOf(context.getFunction().getFunctionName().toUpperCase())) {
+ case SET_DIFF:
+ // set diff can only have 2 arguments
+ if (context.getFunction().getArguments().size() != 2) {
+ throw new IllegalArgumentException("SET_DIFF function can only have
2 arguments.");
+ }
+ break;
+ case SET_UNION:
+ case SET_INTERSECT:
Review comment:
Done
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
