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

TUMBLE/HOP/SESSION_START/END do not resolve time field correctly

    Details

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

      Description

      It seems there is a bug in the resolution logic of SqlToRelConverter.

      The problem is as follows:

      Input: MyTable(INTEGER a, VARCHAR b, BIGINT c, TIMESTAMP proctime, TIMESTAMP rowtime)

      SQL: SELECT COUNT, TUMBLE_START(rowtime, INTERVAL '15' MINUTE), TUMBLE_END(rowtime, INTERVAL '15' MINUTE) FROM MyTable GROUP BY TUMBLE(rowtime, INTERVAL '15' MINUTE)

      Exception:

      java.lang.RuntimeException: while converting TUMBLE_START(`MyTable`.`rowtime`, INTERVAL '15' MINUTE)
      
          at org.apache.calcite.sql2rel.ReflectiveConvertletTable$2.convertCall(ReflectiveConvertletTable.java:134)
          at org.apache.calcite.sql2rel.SqlNodeToRexConverterImpl.convertCall(SqlNodeToRexConverterImpl.java:61)
          at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:4415)
          at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:3783)
          at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:137)
          at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4317)
          at org.apache.calcite.sql2rel.SqlToRelConverter.createAggImpl(SqlToRelConverter.java:2723)
          at org.apache.calcite.sql2rel.SqlToRelConverter.convertAgg(SqlToRelConverter.java:2541)
          at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:654)
          at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:616)
          at org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:2951)
          at org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:552)
          .....
      Caused by: java.lang.reflect.InvocationTargetException
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
          at java.lang.reflect.Method.invoke(Method.java:498)
          at org.apache.calcite.sql2rel.ReflectiveConvertletTable$2.convertCall(ReflectiveConvertletTable.java:131)
          ... 42 more
      Caused by: java.lang.AssertionError
          at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.getRootField(SqlToRelConverter.java:4132)
          at org.apache.calcite.sql2rel.SqlToRelConverter.adjustInputRef(SqlToRelConverter.java:3446)
          at org.apache.calcite.sql2rel.SqlToRelConverter.convertIdentifier(SqlToRelConverter.java:3421)
          at org.apache.calcite.sql2rel.SqlToRelConverter.access$1800(SqlToRelConverter.java:207)
          at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:4424)
          at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java
      

      Additionally, tests were added to
      SqlToRelConverterTest.xml but not to SqlToRelConverterTest.java. We've
      been running without tests.

        Activity

        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Resolved in release 1.13.0 (2017-06-26).

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.13.0 (2017-06-26).
        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/a11d1405 . Thanks for the test case, Haohui Mai !
        Hide
        twalthr Timo Walther added a comment -

        Julian Hyde thanks for looking into this so quickly! Your fix seems to solve the wrong field resolution. The additional PLUS in AuxiliaryConverter breaks our tests but I think this is our problem.

        Show
        twalthr Timo Walther added a comment - Julian Hyde thanks for looking into this so quickly! Your fix seems to solve the wrong field resolution. The additional PLUS in AuxiliaryConverter breaks our tests but I think this is our problem.
        Hide
        julianhyde Julian Hyde added a comment -

        I pushed a fix to https://github.com/julianhyde/calcite/tree/1761-hop. Please let me know whether it solves the issue.

        It's rough; more work is needed on SESSION_END. But hopefully you can see what I mean by "grouped auxiliary function" and the mechanism to compute them from the group by expressions in the Project that follows the Aggregate.

        Show
        julianhyde Julian Hyde added a comment - I pushed a fix to https://github.com/julianhyde/calcite/tree/1761-hop . Please let me know whether it solves the issue. It's rough; more work is needed on SESSION_END. But hopefully you can see what I mean by "grouped auxiliary function" and the mechanism to compute them from the group by expressions in the Project that follows the Aggregate.
        Hide
        julianhyde Julian Hyde added a comment -

        It's complicated. I think it's easier if I just fix the issue.

        Show
        julianhyde Julian Hyde added a comment - It's complicated. I think it's easier if I just fix the issue.
        Hide
        wheat9 Haohui Mai added a comment -

        Thanks for looking into it. I'm not sure I fully understand your comments. I assume at the very end that TUMBLE_START(footime, INTERVAL '1' MINUTE) will be translated to something like:

        RexCall('TUMBLE_START', RexInputRef(footime), RexLiteral(60000))
        

        Since footime does not exist in the root of the blackboard, RexInputRef(footime) will be incorrect regardless what offset it puts in. Is my understanding correct?

        Show
        wheat9 Haohui Mai added a comment - Thanks for looking into it. I'm not sure I fully understand your comments. I assume at the very end that TUMBLE_START(footime, INTERVAL '1' MINUTE) will be translated to something like: RexCall('TUMBLE_START', RexInputRef(footime), RexLiteral(60000)) Since footime does not exist in the root of the blackboard, RexInputRef(footime) will be incorrect regardless what offset it puts in. Is my understanding correct?
        Hide
        julianhyde Julian Hyde added a comment -

        I see that convertAuxiliaryToGroupCall is being called but we don't do anything with the result - we should put it in a list so that it can be found when lookupGroupExpr is called later.

        Show
        julianhyde Julian Hyde added a comment - I see that convertAuxiliaryToGroupCall is being called but we don't do anything with the result - we should put it in a list so that it can be found when lookupGroupExpr is called later.
        Hide
        julianhyde Julian Hyde added a comment -

        Thanks, I can reproduce the problem with that test case. For the record, here's the query:

        select STREAM TUMBLE_START(footime, INTERVAL '1' MINUTE)
        from ORDERS
        GROUP BY TUMBLE(footime, INTERVAL '1' MINUTE)

        The error is occurring while translating the SELECT clause. It is trying to translate the Orders.FOOTIME field, which doesn't exist because it isn't in the GROUP BY. In fact it should have recognized that TUMBLE_START(footime, INTERVAL '1' MINUTE) was a grouping expression (it's not, but near enough - it's derived from the real grouping expression TUMBLE(footime, INTERVAL '1' MINUTE)) and if it had, it would not have recursed in to translate footime.

        So I think the set of auxiliary grouping expressions needs to be populated better.

        Show
        julianhyde Julian Hyde added a comment - Thanks, I can reproduce the problem with that test case. For the record, here's the query: select STREAM TUMBLE_START(footime, INTERVAL '1' MINUTE) from ORDERS GROUP BY TUMBLE(footime, INTERVAL '1' MINUTE) The error is occurring while translating the SELECT clause. It is trying to translate the Orders.FOOTIME field, which doesn't exist because it isn't in the GROUP BY. In fact it should have recognized that TUMBLE_START(footime, INTERVAL '1' MINUTE) was a grouping expression (it's not, but near enough - it's derived from the real grouping expression TUMBLE(footime, INTERVAL '1' MINUTE) ) and if it had, it would not have recursed in to translate footime . So I think the set of auxiliary grouping expressions needs to be populated better.
        Hide
        wheat9 Haohui Mai added a comment -

        Timo Walther and me talked offline and we are both looking at this error.

        I've created a PR (https://github.com/apache/calcite/pull/435) that reproduces the failure. Julian Hyde can you please shed some light on what went wrong?

        Thanks.

        Show
        wheat9 Haohui Mai added a comment - Timo Walther and me talked offline and we are both looking at this error. I've created a PR ( https://github.com/apache/calcite/pull/435 ) that reproduces the failure. Julian Hyde can you please shed some light on what went wrong? Thanks.

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            twalthr Timo Walther
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development