[incubator-pinot] 01/01: Make Compilation time function evaluation after query parser

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xiangfu pushed a commit to branch fixing_function_evaluation
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit 93962cadd881ee5f5ca1b07e4cf4f6cbe333b087
Author: Xiang Fu <fx...@gmail.com>
AuthorDate: Sat May 23 14:32:07 2020 -0700

    Make Compilation time function evaluation after query parser
---
 .../pinot/common/function/FunctionRegistry.java    |  2 +
 .../pinot/common/function/StringFunctions.java     | 35 ++++++++
 .../apache/pinot/sql/parsers/CalciteSqlParser.java | 92 +++++++++++++---------
 .../pinot/sql/parsers/CalciteSqlCompilerTest.java  | 33 +++++++-
 4 files changed, 122 insertions(+), 40 deletions(-)

diff --git a/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java b/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
index a502bf2..d9f7a79 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
@@ -88,6 +88,8 @@ public class FunctionRegistry {
       FunctionRegistry.registerFunction(DateTimeFunctions.class.getDeclaredMethod("now"));
 
       FunctionRegistry.registerFunction(JsonFunctions.class.getDeclaredMethod("toJsonMapStr", Map.class));
+
+      FunctionRegistry.registerFunction(StringFunctions.class.getDeclaredMethod("reverse", String.class));
     } catch (NoSuchMethodException e) {
       LOGGER.error("Caught exception when registering function", e);
       throw new IllegalStateException(e);
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/function/StringFunctions.java b/pinot-common/src/main/java/org/apache/pinot/common/function/StringFunctions.java
new file mode 100644
index 0000000..b57032e
--- /dev/null
+++ b/pinot-common/src/main/java/org/apache/pinot/common/function/StringFunctions.java
@@ -0,0 +1,35 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.function;
+
+import org.apache.commons.lang.StringUtils;
+
+
+/**
+ * Inbuilt string related transform functions
+ *
+ */
+public class StringFunctions {
+  /**
+   * Reverse any given string
+   */
+  static String reverse(String input) {
+    return StringUtils.reverse(input);
+  }
+}
diff --git a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
index a672fc8..d818ea7 100644
--- a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
+++ b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
@@ -318,6 +318,9 @@ public class CalciteSqlParser {
   }
 
   private static void queryRewrite(PinotQuery pinotQuery) {
+    // Invoke compilation time functions
+    invokeCompileTimeFunction(pinotQuery);
+
     // Update Predicate Comparison
     if (pinotQuery.isSetFilterExpression()) {
       Expression filterExpression = pinotQuery.getFilterExpression();
@@ -331,6 +334,25 @@ public class CalciteSqlParser {
     validate(aliasMap, pinotQuery);
   }
 
+  private static void invokeCompileTimeFunction(PinotQuery pinotQuery) {
+    for (int i = 0; i < pinotQuery.getSelectListSize(); i++) {
+      Expression expression = invokeCompileTimeFunctionExpression(pinotQuery.getSelectList().get(i));
+      pinotQuery.getSelectList().set(i, expression);
+    }
+    for (int i = 0; i < pinotQuery.getGroupByListSize(); i++) {
+      Expression expression = invokeCompileTimeFunctionExpression(pinotQuery.getGroupByList().get(i));
+      pinotQuery.getGroupByList().set(i, expression);
+    }
+    for (int i = 0; i < pinotQuery.getOrderByListSize(); i++) {
+      Expression expression = invokeCompileTimeFunctionExpression(pinotQuery.getOrderByList().get(i));
+      pinotQuery.getOrderByList().set(i, expression);
+    }
+    Expression filterExpression = invokeCompileTimeFunctionExpression(pinotQuery.getFilterExpression());
+    pinotQuery.setFilterExpression(filterExpression);
+    Expression havingExpression = invokeCompileTimeFunctionExpression(pinotQuery.getHavingExpression());
+    pinotQuery.setHavingExpression(havingExpression);
+  }
+
   // This method converts a predicate expression to the what Pinot could evaluate.
   // For comparison expression, left operand could be any expression, but right operand only
   // supports literal.
@@ -596,7 +618,7 @@ public class CalciteSqlParser {
           // Move on to process default logic.
         }
       default:
-        return evaluateFunctionExpression((SqlBasicCall) node);
+        return compileFunctionExpression((SqlBasicCall) node);
     }
   }
 
@@ -613,29 +635,9 @@ public class CalciteSqlParser {
     return funcName;
   }
 
-  private static Expression evaluateFunctionExpression(SqlBasicCall funcSqlNode) {
+  private static Expression compileFunctionExpression(SqlBasicCall funcSqlNode) {
     String funcName = extractFunctionName(funcSqlNode);
     Expression funcExpr = RequestUtils.getFunctionExpression(funcName);
-    if (FunctionRegistry.containsFunctionByName(funcName) && isCompileTimeEvaluationPossible(funcExpr)) {
-      int functionOperandsLength = funcSqlNode.getOperands().length;
-      FunctionInfo functionInfo = FunctionRegistry.getFunctionByName(funcName);
-      Object[] arguments = new Object[functionOperandsLength];
-      for (int i = 0; i < functionOperandsLength; i++) {
-        if (funcSqlNode.getOperands()[i] instanceof SqlLiteral) {
-          arguments[i] = ((SqlLiteral) funcSqlNode.getOperands()[i]).toValue();
-        } else {
-          // Evaluate function call (SqlBasicCall) recursively.
-          arguments[i] = evaluateFunctionExpression((SqlBasicCall) funcSqlNode.getOperands()[i]).getLiteral().getFieldValue();
-        }
-      }
-      try {
-        FunctionInvoker invoker = new FunctionInvoker(functionInfo);
-        Object result = invoker.process(arguments);
-        return RequestUtils.getLiteralExpression(result);
-      } catch (Exception e) {
-        throw new SqlCompilationException(new IllegalArgumentException("Unsupported function - " + funcName, e));
-      }
-    }
     for (SqlNode child : funcSqlNode.getOperands()) {
       if (child instanceof SqlNodeList) {
         final Iterator<SqlNode> iterator = ((SqlNodeList) child).iterator();
@@ -649,24 +651,36 @@ public class CalciteSqlParser {
     }
     return funcExpr;
   }
-  /**
-   * Utility method to check if the function can be evaluated during the query compilation phae
-   * @param funcExpr
-   * @return true if all arguments are literals
-   */
-  private static boolean isCompileTimeEvaluationPossible(Expression funcExpr) {
-    Function functionCall = funcExpr.getFunctionCall();
-    if (functionCall.getOperandsSize() > 0) {
-      for (Expression expression : functionCall.getOperands()) {
-        if (expression.getType() == ExpressionType.FUNCTION) {
-          if (!isCompileTimeEvaluationPossible(expression)){
-            return false;
-          }
-        } else if (expression.getType() != ExpressionType.LITERAL) {
-          return false;
-        }
+
+  protected static Expression invokeCompileTimeFunctionExpression(Expression funcExpr) {
+    if (funcExpr == null || funcExpr.getFunctionCall() == null) {
+      return funcExpr;
+    }
+    Function function = funcExpr.getFunctionCall();
+    int functionOperandsLength = function.getOperandsSize();
+    boolean compilable = true;
+    for (int i = 0; i < functionOperandsLength; i++) {
+      Expression operand = invokeCompileTimeFunctionExpression(function.getOperands().get(i));
+      if (operand.getLiteral() == null) {
+        compilable = false;
       }
+      function.getOperands().set(i, operand);
     }
-    return true;
+    String funcName = function.getOperator();
+    if (FunctionRegistry.containsFunctionByName(funcName) && compilable) {
+      FunctionInfo functionInfo = FunctionRegistry.getFunctionByName(funcName);
+      Object[] arguments = new Object[functionOperandsLength];
+      for (int i = 0; i < functionOperandsLength; i++) {
+        arguments[i] = function.getOperands().get(i).getLiteral().getFieldValue();
+      }
+      try {
+        FunctionInvoker invoker = new FunctionInvoker(functionInfo);
+        Object result = invoker.process(arguments);
+        return RequestUtils.getLiteralExpression(result);
+      } catch (Exception e) {
+        throw new SqlCompilationException(new IllegalArgumentException("Unsupported function - " + funcName, e));
+      }
+    }
+    return funcExpr;
   }
 }
diff --git a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
index ff92f2e..1fb57e3 100644
--- a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
+++ b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
@@ -1520,7 +1520,38 @@ public class CalciteSqlCompilerTest {
     PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
     Function greaterThan = pinotQuery.getFilterExpression().getFunctionCall();
     String today = greaterThan.getOperands().get(1).getLiteral().getStringValue();
-    String expectedTodayStr = Instant.now().atZone(ZoneId.of("UTC")).format(DateTimeFormatter.ofPattern("yyyy-MM-dd z"));
+    String expectedTodayStr =
+        Instant.now().atZone(ZoneId.of("UTC")).format(DateTimeFormatter.ofPattern("yyyy-MM-dd z"));
     Assert.assertEquals(today, expectedTodayStr);
   }
+
+  @Test
+  public void testCompileTimeExpression()
+      throws SqlParseException {
+    // True
+    Expression expression = CalciteSqlParser.compileToExpression("now()");
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.invokeCompileTimeFunctionExpression(expression);
+    Assert.assertTrue(expression.getLiteral() != null);
+    expression = CalciteSqlParser.compileToExpression("toDateTime(now(), 'yyyy-MM-dd z')");
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.invokeCompileTimeFunctionExpression(expression);
+    Assert.assertTrue(expression.getLiteral() != null);
+    expression = CalciteSqlParser.compileToExpression("toDateTime(playerName)");
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.invokeCompileTimeFunctionExpression(expression);
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.compileToExpression("reverse(playerName)");
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.invokeCompileTimeFunctionExpression(expression);
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.compileToExpression("reverse('playerName')");
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.invokeCompileTimeFunctionExpression(expression);
+    Assert.assertTrue(expression.getLiteral() != null);
+    expression = CalciteSqlParser.compileToExpression("count(*)");
+    Assert.assertTrue(expression.getFunctionCall() != null);
+    expression = CalciteSqlParser.invokeCompileTimeFunctionExpression(expression);
+    Assert.assertTrue(expression.getFunctionCall() != null);
+  }
 }
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org