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

Reply via email to