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

AssertionError extracting value from an INTERVAL literal

    Details

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

      Description

      CAST(...) * (INTERVAL constant) results into Internal Error.

      For example,

      select cast(empno as Integer) * (INTERVAL '1' DAY)
      from emp
      

      results into

      java.lang.AssertionError: Internal error: invalid literal: INTERVAL '1' DAY
      

      The reason is that INTERVAL constant is not extracted properly in the cases where this constant times a CAST() function

        Issue Links

          Activity

          Hide
          seanhychu Sean Hsuan-Yi Chu added a comment -
          Show
          seanhychu Sean Hsuan-Yi Chu added a comment - Julian Hyde Can you help this patch ? Thanks https://github.com/apache/incubator-calcite/pull/155/commits
          Hide
          seanhychu Sean Hsuan-Yi Chu added a comment -

          Julian Hyde Can you please help review ? Thanks.

          Show
          seanhychu Sean Hsuan-Yi Chu added a comment - Julian Hyde Can you please help review ? Thanks.
          Hide
          julianhyde Julian Hyde added a comment -

          Concur with Vladimir Sitnikov's comments. You should not call getSign(). Change the test to INTERVAL '2' DAY and see whether you get the right result.

          Does select 5 * (INTERVAL '1' DAY) from emp also reproduce the problem? I suspect it does. If so, remove 'CAST' from the description of this case.

          Show
          julianhyde Julian Hyde added a comment - Concur with Vladimir Sitnikov 's comments. You should not call getSign(). Change the test to INTERVAL '2' DAY and see whether you get the right result. Does select 5 * (INTERVAL '1' DAY) from emp also reproduce the problem? I suspect it does. If so, remove 'CAST' from the description of this case.
          Hide
          seanhychu Sean Hsuan-Yi Chu added a comment -

          1. Now I am addressing Vladimir Sitnikov comment. Thanks.

          2. "Change the test to INTERVAL '2' DAY" would reproduce the same InternalError.

          3. Actually, constants on both sides of '*' would NOT reproduce this error.
          The reason is: in SqlMonotonicBinaryOperator.getMonotonicity(), Calcite would take a different path if both sides of operator(in this case, it is '*') are constants.

          Show
          seanhychu Sean Hsuan-Yi Chu added a comment - 1. Now I am addressing Vladimir Sitnikov comment. Thanks. 2. "Change the test to INTERVAL '2' DAY" would reproduce the same InternalError. 3. Actually, constants on both sides of '*' would NOT reproduce this error. The reason is: in SqlMonotonicBinaryOperator.getMonotonicity(), Calcite would take a different path if both sides of operator(in this case, it is '*') are constants.
          Hide
          seanhychu Sean Hsuan-Yi Chu added a comment -

          Responding to the review comment, for INTERVAL_YEAR_MONTH or INTERVAL_DAY_TIME, I let the value() method return "sign multiplied by its value in milli-second".

          Please see the pull request: https://github.com/apache/incubator-calcite/pull/155/commits

          Show
          seanhychu Sean Hsuan-Yi Chu added a comment - Responding to the review comment, for INTERVAL_YEAR_MONTH or INTERVAL_DAY_TIME, I let the value() method return "sign multiplied by its value in milli-second". Please see the pull request: https://github.com/apache/incubator-calcite/pull/155/commits
          Hide
          julianhyde Julian Hyde added a comment -

          Looks mostly good. However, I added a test to misc.oq (run via JdbcTest.testRunMisc):

          # [CALCITE-922] Value of INTERVAL literal
          select deptno * interval '2' day as d2,
           deptno * interval -'3' hour as h3
          from "scott".dept;
          +-----+------+
          | D2  | H3   |
          +-----+------+
          | +20 | -30  |
          | +40 | -60  |
          | +60 | -90  |
          | +80 | -120 |
          +-----+------+
          (4 rows)
          !ok

          (also in https://github.com/julianhyde/incubator-calcite/tree/922-interval-literals)
          and it gave the wrong results for negative intervals. E.g. h3 in the first row was +30, should be -30.

          I saw your discussion about negative intervals on the dev list and I think we should fix those as part of this issue.

          Show
          julianhyde Julian Hyde added a comment - Looks mostly good. However, I added a test to misc.oq (run via JdbcTest.testRunMisc): # [CALCITE-922] Value of INTERVAL literal select deptno * interval '2' day as d2, deptno * interval -'3' hour as h3 from "scott".dept; +-----+------+ | D2 | H3 | +-----+------+ | +20 | -30 | | +40 | -60 | | +60 | -90 | | +80 | -120 | +-----+------+ (4 rows) !ok (also in https://github.com/julianhyde/incubator-calcite/tree/922-interval-literals ) and it gave the wrong results for negative intervals. E.g. h3 in the first row was +30, should be -30. I saw your discussion about negative intervals on the dev list and I think we should fix those as part of this issue.
          Hide
          seanhychu Sean Hsuan-Yi Chu added a comment -

          I updated the pull request. The new one can resolve this case.

          But in your patch, you might need to insert a white space between "(4 rows)" and "!ok".

          Otherwise, the tester will complain the output files failing to match.

          Show
          seanhychu Sean Hsuan-Yi Chu added a comment - I updated the pull request. The new one can resolve this case. But in your patch, you might need to insert a white space between "(4 rows)" and "!ok". Otherwise, the tester will complain the output files failing to match.
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/c5f2599f . Thanks for the patch, Sean Hsuan-Yi Chu !
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          Resolved in release 1.5.0 (2015-11-10)

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.5.0 (2015-11-10)

            People

            • Assignee:
              seanhychu Sean Hsuan-Yi Chu
              Reporter:
              seanhychu Sean Hsuan-Yi Chu
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development