You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xi...@apache.org on 2022/02/23 09:59:20 UTC

[pinot] branch master updated: [issue-8157] upgrade calcite due to cve (#8158)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 7288509  [issue-8157] upgrade calcite due to cve (#8158)
7288509 is described below

commit 72885092319bdf38c0a6c9b0836cb9b1e12a5091
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Wed Feb 23 10:58:50 2022 +0100

    [issue-8157] upgrade calcite due to cve (#8158)
    
    * [issue-8157] upgrade calcite due to cve
    
    * add workaround suggested by @xiangfu0
    
    * Update pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
    
    Co-authored-by: Rong Rong <wa...@gmail.com>
    
    * Adding check for invalid distinct rewrite
    
    Co-authored-by: Rong Rong <wa...@gmail.com>
    Co-authored-by: Xiang Fu <xi...@gmail.com>
---
 LICENSE-binary                                     |  6 ++--
 .../apache/pinot/sql/parsers/CalciteSqlParser.java | 38 ++++++++++++++--------
 .../pinot/sql/parsers/CalciteSqlCompilerTest.java  | 11 +++++++
 pom.xml                                            |  6 +++-
 4 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/LICENSE-binary b/LICENSE-binary
index 2af9676..02af00d 100644
--- a/LICENSE-binary
+++ b/LICENSE-binary
@@ -326,9 +326,9 @@ org.apache.avro:avro:1.9.2
 org.apache.bookkeeper:bookkeeper-common-allocator:4.12.0
 org.apache.bookkeeper:circe-checksum:4.12.0
 org.apache.calcite.avatica:avatica-core:1.13.0
-org.apache.calcite:calcite-babel:1.19.0
-org.apache.calcite:calcite-core:1.19.0
-org.apache.calcite:calcite-linq4j:1.19.0
+org.apache.calcite:calcite-babel:1.29.0
+org.apache.calcite:calcite-core:1.29.0
+org.apache.calcite:calcite-linq4j:1.29.0
 org.apache.commons:commons-collections4:4.1
 org.apache.commons:commons-compress:1.20
 org.apache.commons:commons-csv:1.0
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 107c1ce..5c21ef0 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
@@ -559,7 +559,7 @@ public class CalciteSqlParser {
       case DESCENDING:
         SqlBasicCall basicCall = (SqlBasicCall) node;
         expression = RequestUtils.getFunctionExpression("DESC");
-        expression.getFunctionCall().addToOperands(toExpression(basicCall.getOperands()[0]));
+        expression.getFunctionCall().addToOperands(toExpression(basicCall.getOperandList().get(0)));
         break;
       case IDENTIFIER:
       default:
@@ -615,8 +615,9 @@ public class CalciteSqlParser {
         return RequestUtils.getLiteralExpression((SqlLiteral) node);
       case AS:
         SqlBasicCall asFuncSqlNode = (SqlBasicCall) node;
-        Expression leftExpr = toExpression(asFuncSqlNode.getOperands()[0]);
-        SqlNode aliasSqlNode = asFuncSqlNode.getOperands()[1];
+        List<SqlNode> operands = asFuncSqlNode.getOperandList();
+        Expression leftExpr = toExpression(operands.get(0));
+        SqlNode aliasSqlNode = operands.get(1);
         String aliasName;
         switch (aliasSqlNode.getKind()) {
           case IDENTIFIER:
@@ -710,14 +711,23 @@ public class CalciteSqlParser {
           compilePathExpression(functionName, functionNode, path);
           return RequestUtils.getIdentifierExpression(path.toString());
         }
+        if ((functionNode.getFunctionQuantifier() != null) && ("DISTINCT".equals(
+            functionNode.getFunctionQuantifier().toString()))) {
+          if (AggregationFunctionType.COUNT.name().equals(functionName)) {
+            functionName = AggregationFunctionType.DISTINCTCOUNT.name();
+          } else if (AggregationFunctionType.isAggregationFunction(functionName)) {
+            // Aggregation function(other than COUNT) on DISTINCT is not supported, e.g. SUM(DISTINCT colA).
+            throw new SqlCompilationException("Function '" + functionName + "' on DISTINCT is not supported.");
+          }
+        }
         break;
       default:
         functionName = functionKind.name();
         break;
     }
     // When there is no argument, set an empty list as the operands
-    SqlNode[] childNodes = functionNode.getOperands();
-    List<Expression> operands = new ArrayList<>(childNodes.length);
+    List<SqlNode> childNodes = functionNode.getOperandList();
+    List<Expression> operands = new ArrayList<>(childNodes.size());
     for (SqlNode childNode : childNodes) {
       if (childNode instanceof SqlNodeList) {
         for (SqlNode node : (SqlNodeList) childNode) {
@@ -754,14 +764,14 @@ public class CalciteSqlParser {
    * @param path String representation of path represented by DOT and/or ITEM function chain.
    */
   private static void compilePathExpression(String functionName, SqlBasicCall functionNode, StringBuffer path) {
-    SqlNode[] operands = functionNode.getOperands();
+    List<SqlNode> operands = functionNode.getOperandList();
 
     // Compile first operand of the function (either an identifier or another DOT and/or ITEM function).
-    SqlKind kind0 = operands[0].getKind();
+    SqlKind kind0 = operands.get(0).getKind();
     if (kind0 == SqlKind.IDENTIFIER) {
-      path.append(operands[0].toString());
+      path.append(operands.get(0).toString());
     } else if (kind0 == SqlKind.DOT || kind0 == SqlKind.OTHER_FUNCTION) {
-      SqlBasicCall function0 = (SqlBasicCall) operands[0];
+      SqlBasicCall function0 = (SqlBasicCall) operands.get(0);
       String name0 = function0.getOperator().getName();
       if (name0.equals("ITEM") || name0.equals("DOT")) {
         compilePathExpression(name0, function0, path);
@@ -773,11 +783,11 @@ public class CalciteSqlParser {
     }
 
     // Compile second operand of the function (either an identifier or literal).
-    SqlKind kind1 = operands[1].getKind();
+    SqlKind kind1 = operands.get(1).getKind();
     if (kind1 == SqlKind.IDENTIFIER) {
-      path.append(".").append(((SqlIdentifier) operands[1]).getSimple());
+      path.append(".").append(((SqlIdentifier) operands.get(1)).getSimple());
     } else if (kind1 == SqlKind.LITERAL) {
-      path.append("[").append(((SqlLiteral) operands[1]).toValue()).append("]");
+      path.append("[").append(((SqlLiteral) operands.get(1)).toValue()).append("]");
     } else {
       throw new SqlCompilationException("SELECT list item has bad path expression.");
     }
@@ -839,7 +849,7 @@ public class CalciteSqlParser {
    */
   private static Expression compileAndExpression(SqlBasicCall andNode) {
     List<Expression> operands = new ArrayList<>();
-    for (SqlNode childNode : andNode.getOperands()) {
+    for (SqlNode childNode : andNode.getOperandList()) {
       if (childNode.getKind() == SqlKind.AND) {
         Expression childAndExpression = compileAndExpression((SqlBasicCall) childNode);
         operands.addAll(childAndExpression.getFunctionCall().getOperands());
@@ -857,7 +867,7 @@ public class CalciteSqlParser {
    */
   private static Expression compileOrExpression(SqlBasicCall orNode) {
     List<Expression> operands = new ArrayList<>();
-    for (SqlNode childNode : orNode.getOperands()) {
+    for (SqlNode childNode : orNode.getOperandList()) {
       if (childNode.getKind() == SqlKind.OR) {
         Expression childAndExpression = compileOrExpression((SqlBasicCall) childNode);
         operands.addAll(childAndExpression.getFunctionCall().getOperands());
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 e706de9..3523c95 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
@@ -1671,6 +1671,17 @@ public class CalciteSqlCompilerTest {
   }
 
   @Test
+  public void testInvalidDistinctCountRewrite() {
+    String query = "SELECT sum(distinct bar) FROM foo";
+    try {
+      PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+    } catch (Exception e) {
+      Assert.assertTrue(e instanceof SqlCompilationException);
+      Assert.assertEquals(e.getMessage(), "Function 'SUM' on DISTINCT is not supported.");
+    }
+  }
+
+  @Test
   public void testOrdinalsQueryRewrite() {
     String query = "SELECT foo, bar, count(*) FROM t GROUP BY 1, 2 ORDER BY 1, 2 DESC";
     PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
diff --git a/pom.xml b/pom.xml
index 0b95efb..8d639de 100644
--- a/pom.xml
+++ b/pom.xml
@@ -132,7 +132,7 @@
     <antlr.version>4.6</antlr.version>
     <jsonpath.version>2.7.0</jsonpath.version>
     <quartz.version>2.3.2</quartz.version>
-    <calcite.version>1.19.0</calcite.version>
+    <calcite.version>1.29.0</calcite.version>
     <lucene.version>8.2.0</lucene.version>
     <reflections.version>0.9.9</reflections.version>
     <!-- commons-configuration, hadoop-common, hadoop-client use commons-lang -->
@@ -1095,6 +1095,10 @@
             <groupId>com.google.code.findbugs</groupId>
             <artifactId>jsr305</artifactId>
           </exclusion>
+          <exclusion>
+            <groupId>com.google.errorprone</groupId>
+            <artifactId>error_prone_annotations</artifactId>
+          </exclusion>
         </exclusions>
       </dependency>
       <dependency>

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