vvivekiyer commented on code in PR #8518:
URL: https://github.com/apache/pinot/pull/8518#discussion_r849963166
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/PredicateComparisonRewriter.java:
##########
@@ -42,6 +44,73 @@ public PinotQuery rewrite(PinotQuery pinotQuery) {
return pinotQuery;
}
+ /**
+ * Updates boolean predicates that are missing an EQUALS filter.
+ * E.g. 1: 'WHERE a' will be updated to 'WHERE a = true'
+ * E.g. 2: "WHERE startsWith(col, 'str')" will be updated to "WHERE
startsWith(col, 'str') = true"
+ *
+ * @param expression current expression in the expression tree
+ * @param parentFunction parent expression
+ * @return re-written expression.
+ */
+ private static Expression updateBooleanPredicates(Expression expression,
Function parentFunction) {
+ ExpressionType type = expression.getType();
+ if (type == ExpressionType.FUNCTION) {
+ Function function = expression.getFunctionCall();
+
+ // If the function is not of FilterKind, we might have to rewrite the
function if parentFunction is AND, OR or
+ // NOT. Example: A query like "select col1 from table where
startsWith(col1, 'myStr') AND col2 > 10;" should be
+ // rewritten to "select col1 from table where startsWith(col1, 'myStr')
= true AND col2 > 10;".
+ if (!EnumUtils.isValidEnum(FilterKind.class, function.getOperator())) {
+
+ // Rewrite non FilterKind functions if one of the two conditions are
satified:
+ // 1. parent function is empty. Example: "select * from table where
startsWith(col, 'myStr');
+ // 2. Separator is not a predicate (non predicates are AND, OR, NOT).
+ // Example: "select * from table where startsWith(col, 'myStr') AND
col2 > 10";
+ if (parentFunction == null || (EnumUtils.isValidEnum(FilterKind.class,
parentFunction.getOperator())
+ &&
!FilterKind.valueOf(parentFunction.getOperator()).isPredicate())) {
+ Expression currExpression = expression.deepCopy();
+ expression =
RequestUtils.getFunctionExpression(FilterKind.EQUALS.name());
+ List<Expression> operands = new ArrayList<>();
+ operands.add(currExpression);
+ operands.add(RequestUtils.getLiteralExpression(true));
+ expression.getFunctionCall().setOperands(operands);
+
+ return expression;
+ }
+ }
+
+ List<Expression> operands = expression.getFunctionCall().getOperands();
+
+ if (operands.size() > 0) {
+ // unset operands only within the "if" condition because 'no argument'
functions expect operand list to
+ // be initialized.
+ expression.getFunctionCall().unsetOperands();
+
+ for (Expression exp : operands) {
+
expression.getFunctionCall().addToOperands(updateBooleanPredicates(exp,
expression.getFunctionCall()));
+ }
+ }
+ } else if (type == ExpressionType.IDENTIFIER) {
+
+ // Rewrite identifiers if one of the two conditions are satified:
+ // 1. parent function is empty. Example: "select * from table where
col1";
+ // 2. Separator is not a predicate (non predicates are AND, OR, NOT).
+ // Example: "select * from table where col1 AND col2 > 10";
+ if (parentFunction == null || (EnumUtils.isValidEnum(FilterKind.class,
parentFunction.getOperator())
Review Comment:
In both the above example, when we reach `WHERE col1` we have to rewrite the
query.
But consider the example query `WHERE concat(col1, "str")`. In this case we
have to not rewrite the query.
So the logic is that, literals will be re-written only if at least one of
the conditions is satisfied:
1. Parent function is null.
2. Parent function is one of AND, OR, NOT.
##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java:
##########
@@ -2246,6 +2246,19 @@ public void testFlattenAndOr() {
}
}
+ {
+ String query = "SELECT * FROM foo WHERE col1 > 0 AND (col2 AND col3 > 0)
AND startsWith(col4, 'myStr')";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ Function functionCall =
pinotQuery.getFilterExpression().getFunctionCall();
+ Assert.assertEquals(functionCall.getOperator(), FilterKind.AND.name());
+ List<Expression> operands = functionCall.getOperands();
+ Assert.assertEquals(operands.size(), 4);
+ Assert.assertEquals(operands.get(0).getFunctionCall().getOperator(),
FilterKind.GREATER_THAN.name());
+ Assert.assertEquals(operands.get(1).getFunctionCall().getOperator(),
FilterKind.EQUALS.name());
+ Assert.assertEquals(operands.get(2).getFunctionCall().getOperator(),
FilterKind.GREATER_THAN.name());
+ Assert.assertEquals(operands.get(3).getFunctionCall().getOperator(),
FilterKind.EQUALS.name());
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.
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
