[incubator-pinot] branch master updated: Mitigate calcite NPE bug. (#6908)

[siddteotia]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 59e342d  Mitigate calcite NPE bug. (#6908)
59e342d is described below

commit 59e342df9ba7895e1d404c7f681b7c8ab664d7de
Author: Amrish Lal <amrish.k....@gmail.com>
AuthorDate: Fri May 14 12:02:24 2021 -0700

    Mitigate calcite NPE bug. (#6908)
---
 .../org/apache/pinot/common/utils/request/RequestUtils.java  |  6 +++++-
 .../org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java | 12 ++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git 
a/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
 
b/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
index f23e670..bcb853a 100644
--- 
a/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
+++ 
b/pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
@@ -107,7 +107,11 @@ public class RequestUtils {
     Expression expression = new Expression(ExpressionType.LITERAL);
     Literal literal = new Literal();
     if (node instanceof SqlNumericLiteral) {
-      if (((SqlNumericLiteral) node).isInteger()) {
+      // Mitigate calcite NPE bug, we need to check if 
SqlNumericLiteral.getScale() is null before calling
+      // SqlNumericLiteral.isInteger(). TODO: Undo this fix once a Calcite 
release that contains CALCITE-4199 is
+      // available and Pinot has been upgraded to use such a release.
+      SqlNumericLiteral sqlNumericLiteral = (SqlNumericLiteral) node;
+      if (sqlNumericLiteral.getScale() != null && 
sqlNumericLiteral.isInteger()) {
         literal.setLongValue(node.bigDecimalValue().longValue());
       } else {
         literal.setDoubleValue(node.bigDecimalValue().doubleValue());
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 64f0331..e100dd7 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
@@ -25,6 +25,7 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNumericLiteral;
 import org.apache.calcite.sql.parser.SqlParseException;
 import org.apache.pinot.common.request.AggregationInfo;
 import org.apache.pinot.common.request.BrokerRequest;
@@ -2185,6 +2186,17 @@ public class CalciteSqlCompilerTest {
         
pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getLiteral().getLongValue(),
 1L);
   }
 
+  /**
+   * This test shows that Calcite {@link SqlNumericLiteral#isInteger()} throws 
NPE. The issue has been fixed in
+   * Calcite through CALCITE-4199 
(https://issues.apache.org/jira/browse/CALCITE-4199), but has not made it into a
+   * release yet.
+   */
+  @Test
+  public void testSqlNumericalLiteralisIntegerNPE() {
+    CalciteSqlCompiler compiler = new CalciteSqlCompiler();
+    BrokerRequest sqlBrokerRequest = compiler.compileToBrokerRequest("SELECT * 
FROM testTable WHERE floatColumn > " + Double.MAX_VALUE);
+  }
+
   @Test
   public void testUnsupportedDistinctQueries() {
     String sql = "SELECT DISTINCT col1, col2 FROM foo GROUP BY col1";

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