vvivekiyer commented on code in PR #8518: URL: https://github.com/apache/pinot/pull/8518#discussion_r860320909
########## pinot-common/src/main/java/org/apache/pinot/common/request/context/RequestContextUtils.java: ########## @@ -166,91 +168,119 @@ public static FunctionContext getFunction(FunctionCallAstNode astNode) { * always convert the right-hand side expressions into strings. */ public static FilterContext getFilter(Expression thriftExpression) { - Function thriftFunction = thriftExpression.getFunctionCall(); - FilterKind filterKind = FilterKind.valueOf(thriftFunction.getOperator().toUpperCase()); - List<Expression> operands = thriftFunction.getOperands(); - int numOperands = operands.size(); - switch (filterKind) { - case AND: - List<FilterContext> children = new ArrayList<>(numOperands); - for (Expression operand : operands) { - children.add(getFilter(operand)); - } - return new FilterContext(FilterContext.Type.AND, children, null); - case OR: - children = new ArrayList<>(numOperands); - for (Expression operand : operands) { - children.add(getFilter(operand)); - } - return new FilterContext(FilterContext.Type.OR, children, null); - case NOT: - assert numOperands == 1; - return new FilterContext(FilterContext.Type.NOT, new ArrayList<>(Collections.singletonList(getFilter(operands.get(0)))), null); - case EQUALS: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new EqPredicate(getExpression(operands.get(0)), getStringValue(operands.get(1)))); - case NOT_EQUALS: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new NotEqPredicate(getExpression(operands.get(0)), getStringValue(operands.get(1)))); - case IN: - List<String> values = new ArrayList<>(numOperands - 1); - for (int i = 1; i < numOperands; i++) { - values.add(getStringValue(operands.get(i))); - } - return new FilterContext(FilterContext.Type.PREDICATE, null, - new InPredicate(getExpression(operands.get(0)), values)); - case NOT_IN: - values = new ArrayList<>(numOperands - 1); - for (int i = 1; i < numOperands; i++) { - values.add(getStringValue(operands.get(i))); - } - return new FilterContext(FilterContext.Type.PREDICATE, null, - new NotInPredicate(getExpression(operands.get(0)), values)); - case GREATER_THAN: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RangePredicate(getExpression(operands.get(0)), false, getStringValue(operands.get(1)), false, - RangePredicate.UNBOUNDED)); - case GREATER_THAN_OR_EQUAL: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RangePredicate(getExpression(operands.get(0)), true, getStringValue(operands.get(1)), false, - RangePredicate.UNBOUNDED)); - case LESS_THAN: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RangePredicate(getExpression(operands.get(0)), false, RangePredicate.UNBOUNDED, false, - getStringValue(operands.get(1)))); - case LESS_THAN_OR_EQUAL: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RangePredicate(getExpression(operands.get(0)), false, RangePredicate.UNBOUNDED, true, - getStringValue(operands.get(1)))); - case BETWEEN: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RangePredicate(getExpression(operands.get(0)), true, getStringValue(operands.get(1)), true, - getStringValue(operands.get(2)))); - case RANGE: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RangePredicate(getExpression(operands.get(0)), getStringValue(operands.get(1)))); - case REGEXP_LIKE: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RegexpLikePredicate(getExpression(operands.get(0)), getStringValue(operands.get(1)))); - case LIKE: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new RegexpLikePredicate(getExpression(operands.get(0)), - RegexpPatternConverterUtils.likeToRegexpLike(getStringValue(operands.get(1))))); - case TEXT_MATCH: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new TextMatchPredicate(getExpression(operands.get(0)), getStringValue(operands.get(1)))); - case JSON_MATCH: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new JsonMatchPredicate(getExpression(operands.get(0)), getStringValue(operands.get(1)))); - case IS_NULL: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new IsNullPredicate(getExpression(operands.get(0)))); - case IS_NOT_NULL: - return new FilterContext(FilterContext.Type.PREDICATE, null, - new IsNotNullPredicate(getExpression(operands.get(0)))); - default: - throw new IllegalStateException(); + return getFilterHelper(thriftExpression, null); + } + + private static FilterContext getFilterHelper(Expression thriftExpression, Function parentFunction) { Review Comment: I added it as a safeguard for future-proofing. It is true that we never call when parentFunction is a predicate. Hence I throw an exception when parentFunction is a predicate. If it's not needed, I can keep the existing function definition as it is currently and make the changes. Thoughts? -- 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