mayankshriv commented on a change in pull request #5832:
URL: https://github.com/apache/incubator-pinot/pull/5832#discussion_r468141336
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java
##########
@@ -108,35 +136,24 @@ public
DistinctCountThetaSketchAggregationFunction(List<ExpressionContext> argum
Preconditions.checkArgument(paramsExpression.getType() ==
ExpressionContext.Type.LITERAL,
"Last argument of DistinctCountThetaSketch must be literal
(post-aggregation expression)");
_postAggregationExpression = QueryContextConverterUtils
-
.getFilter(CalciteSqlParser.compileToExpression(postAggregationExpression.getLiteral()));
+
.getExpression(CalciteSqlParser.compileToExpression(postAggregationExpression.getLiteral()));
// Initialize the predicate map
_predicateInfoMap = new HashMap<>();
- if (numArguments > 3) {
- // Predicates are explicitly specified
- for (int i = 2; i < numArguments - 1; i++) {
- ExpressionContext predicateExpression = arguments.get(i);
- Preconditions.checkArgument(predicateExpression.getType() ==
ExpressionContext.Type.LITERAL,
- "Third to second last argument of DistinctCountThetaSketch must be
literal (predicate expression)");
- Predicate predicate = getPredicate(predicateExpression.getLiteral());
- _inputExpressions.add(predicate.getLhs());
- _predicateInfoMap.put(predicate, new PredicateInfo(predicate));
- }
- } else {
- // Auto-derive predicates from the post-aggregation expression
- Stack<FilterContext> stack = new Stack<>();
- stack.push(_postAggregationExpression);
- while (!stack.isEmpty()) {
- FilterContext filter = stack.pop();
- if (filter.getType() == FilterContext.Type.PREDICATE) {
- Predicate predicate = filter.getPredicate();
- _inputExpressions.add(predicate.getLhs());
- _predicateInfoMap.put(predicate, new PredicateInfo(predicate));
- } else {
- stack.addAll(filter.getChildren());
- }
- }
+
+ // Predicates are explicitly specified
Review comment:
It seems to me that we can always support this in future (if needed,
and/or asked by users) without breaking compatibility with syntax in this PR.
So I am fine with not supporting auto-derive for now.
----------------------------------------------------------------
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]
