Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1595

RelBuilder.call throws NullPointerException if argument types are invalid

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.12.0
    • Component/s: core
    • Labels:
      None

      Description

      I have some RelBuilder code building an expression something like +(+($0, "30"), "30"). Yes, they are string literals This results in an NPE as the type of +($0, "30") is null and it hits the following code:

        public static boolean isExactNumeric(RelDataType type) {
          SqlTypeName typeName = type.getSqlTypeName();
          if (typeName == null) {
            return false;
          }
          switch (typeName) {
      

      Here's the stack:

      	  at org.apache.calcite.sql.type.SqlTypeUtil.isExactNumeric(SqlTypeUtil.java:414)
      	  at org.apache.calcite.rel.type.RelDataTypeFactoryImpl.createDecimalQuotient(RelDataTypeFactoryImpl.java:521)
      	  at org.apache.calcite.sql.type.ReturnTypes$7.inferReturnType(ReturnTypes.java:464)
      	  at org.apache.calcite.sql.type.SqlTypeTransformCascade.inferReturnType(SqlTypeTransformCascade.java:54)
      	  at org.apache.calcite.sql.type.SqlReturnTypeInferenceChain.inferReturnType(SqlReturnTypeInferenceChain.java:56)
      	  at org.apache.calcite.sql.SqlOperator.inferReturnType(SqlOperator.java:469)
      	  at org.apache.calcite.rex.RexBuilder.deriveReturnType(RexBuilder.java:271)
      	  at org.apache.calcite.rex.RexBuilder.makeCall(RexBuilder.java:245)
      	  at org.apache.calcite.tools.RelBuilder.call(RelBuilder.java:529)
      

      Where would be an appropriate place to add a new test for this? I'll get a PR for this and the other stuff created soon.

      Thx.

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          In RexNode land, the arguments to operators have to be the exact type required. The "+" operator, SqlStdOperatorTable.PLUS, requires its types to be numeric. Passing an operand of type CHAR or VARCHAR (if that's what you're doing) is user error. Ideally, RelBuilder should give an exception. But in any case, SqlTypeUtil is not what needs to be fixed.

          Show
          julianhyde Julian Hyde added a comment - In RexNode land, the arguments to operators have to be the exact type required. The "+" operator, SqlStdOperatorTable.PLUS, requires its types to be numeric. Passing an operand of type CHAR or VARCHAR (if that's what you're doing) is user error. Ideally, RelBuilder should give an exception. But in any case, SqlTypeUtil is not what needs to be fixed.
          Hide
          jbalint@gmail.com Jess Balint added a comment -

          Thanks. I'll look for a way to validate it there.

          Show
          jbalint@gmail.com Jess Balint added a comment - Thanks. I'll look for a way to validate it there.
          Hide
          julianhyde Julian Hyde added a comment -

          By the way, you should cast the argument. If you call RelBuilder.cast on a RexNode that happens to be a literal, it will convert the literal to another literal of the same type, if it can. For example, RexCall(CAST, INTEGER, RexLiteral(CHAR, "123") becomes RexLiteral(INTEGER, 123) but RexCall(CAST, INTEGER, RexLiteral(CHAR, "foo bar") remains unchanged and will give a runtime error.

          Show
          julianhyde Julian Hyde added a comment - By the way, you should cast the argument. If you call RelBuilder.cast on a RexNode that happens to be a literal, it will convert the literal to another literal of the same type, if it can. For example, RexCall(CAST, INTEGER, RexLiteral(CHAR, "123") becomes RexLiteral(INTEGER, 123) but RexCall(CAST, INTEGER, RexLiteral(CHAR, "foo bar") remains unchanged and will give a runtime error.
          Hide
          jbalint@gmail.com Jess Balint added a comment -

          Thanks Julian. Looks like a minor inconsistency in the validation. PR submitted.

          Show
          jbalint@gmail.com Jess Balint added a comment - Thanks Julian. Looks like a minor inconsistency in the validation. PR submitted.
          Hide
          julianhyde Julian Hyde added a comment -
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/a55e5eb4 . Thanks for the PR, Jess Balint !
          Hide
          julianhyde Julian Hyde added a comment -

          Resolved in release 1.12.0 (2017-03-24).

          Show
          julianhyde Julian Hyde added a comment - Resolved in release 1.12.0 (2017-03-24).

            People

            • Assignee:
              julianhyde Julian Hyde
              Reporter:
              jbalint@gmail.com Jess Balint
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development