[GitHub] [incubator-pinot] amrishlal commented on a change in pull request #6908: Mitigate calcite NPE bug.

[GitBox <gi...@apache.org>]

amrishlal commented on a change in pull request #6908:
URL: https://github.com/apache/incubator-pinot/pull/6908#discussion_r632708438



##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
##########
@@ -107,7 +107,11 @@ public static Expression getLiteralExpression(SqlLiteral node) {
     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()) {

Review comment:
       Apparently, this happens when Calcite Parser thinks a literal is:
   ```
       <APPROX_NUMERIC_LITERAL> {
           return SqlLiteral.createApproxNumeric(token.image, getPos());
       }
   ```
   where an APPROX_NUMERIC_LITERAL is any numeric value with an exponent:
   ```
   < APPROX_NUMERIC_LITERAL:
       (<UNSIGNED_INTEGER_LITERAL> | <DECIMAL_NUMERIC_LITERAL>) <EXPONENT> >
   ```
   
   From what I can tell, `SqlLiteral.java` class in Calcite needs to be fixed as well (but the bug won't appear in their master branch since they have already added a null check). I will ping their mailing list and see if this is something that should be fixed.
   
   ```
    CURRENT:
     public static SqlNumericLiteral createApproxNumeric(String s, SqlParserPos pos) {
       BigDecimal value = SqlParserUtil.parseDecimal(s);
       return new SqlNumericLiteral(value, (Integer)null, (Integer)null, false, pos);
     }
     
    FIXED:
     public static SqlNumericLiteral createApproxNumeric(String s, SqlParserPos pos) {
       BigDecimal value = SqlParserUtil.parseDecimal(s);
       return new SqlNumericLiteral(value, value.precision(), value.scale(), false, pos);
     }
   
   ```
   But it will probably take a while for these fixes to show up in their release, so we should still mitigate the issue in Pinot.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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