Details

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

      Description

      The following test fails with exception being added to JdbcTest:

        @Test public void testExtractMonthFromTimestamp() {
          CalciteAssert.that()
              .with(CalciteAssert.Config.JDBC_FOODMART)
              .query("select extract(month from \"birth_date\") as c \n"
                  + "from \"foodmart\".\"employee\" where \"employee_id\"=1")
              .returns("C=8\n");
        }
      

      Note, that it is important to make select from real table and not using limit so that planner tries to push EXTRACT to JdbcProject. But then it throws the following exception ():

      Caused by: java.sql.SQLException: Error while executing SQL "select extract(month from "birth_date") as c 
      from "foodmart"."employee" where "employee_id"=1": class org.apache.calcite.sql.SqlSyntax$6: SPECIAL
      	at org.apache.calcite.avatica.Helper.createException(Helper.java:56)
      	at org.apache.calcite.avatica.Helper.createException(Helper.java:41)
      	at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:156)
      	at org.apache.calcite.avatica.AvaticaStatement.executeQuery(AvaticaStatement.java:218)
      	at org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAssert.java:564)
      	... 26 more
      Caused by: java.lang.UnsupportedOperationException: class org.apache.calcite.sql.SqlSyntax$6: SPECIAL
      	at org.apache.calcite.util.Util.needToImplement(Util.java:923)
      	at org.apache.calcite.sql.SqlSyntax$6.unparse(SqlSyntax.java:116)
      	at org.apache.calcite.sql.SqlOperator.unparse(SqlOperator.java:332)
      	at org.apache.calcite.sql.SqlDialect$BaseHandler.unparseCall(SqlDialect.java:733)
      ...
      

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          Resolved in release 1.15.0 (2017-12-11).

          Show
          julianhyde Julian Hyde added a comment - Resolved in release 1.15.0 (2017-12-11).
          Hide
          julianhyde Julian Hyde added a comment -

          Oops, I meant thanks Pavel Gubin.

          Show
          julianhyde Julian Hyde added a comment - Oops, I meant thanks Pavel Gubin .
          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/796a28f9 ; thanks for the PR, Pawel Ruchaj !
          Hide
          julianhyde Julian Hyde added a comment -

          Pavel Gubin, Looks good. I will commit after 1.14 is released.

          Show
          julianhyde Julian Hyde added a comment - Pavel Gubin , Looks good. I will commit after 1.14 is released.
          Hide
          Pavel Gubin Pavel Gubin added a comment -

          All done in PR. Druid integration tests were also checked and amended.

          Show
          Pavel Gubin Pavel Gubin added a comment - All done in PR. Druid integration tests were also checked and amended.
          Hide
          julianhyde Julian Hyde added a comment -

          Based on the discussion in https://github.com/apache/calcite/pull/539 it seems that Michael Mior is +1.

          Reviewing https://github.com/apache/calcite/pull/539/commits/34c33095cdb77f29cf46cae7f40a52feb42751d3, the change looks good. However I gather that there are test failures, so until those changes are committed we're not seeing the full impact. Also, if as you say, EXTRACT_DATE is obsolete, then we should remove it from the code base.

          I see that the Druid adapter makes use of EXTRACT_DATE, so we will need the Druid integration tests to pass before accepting this change.

          Show
          julianhyde Julian Hyde added a comment - Based on the discussion in https://github.com/apache/calcite/pull/539 it seems that Michael Mior is +1. Reviewing https://github.com/apache/calcite/pull/539/commits/34c33095cdb77f29cf46cae7f40a52feb42751d3 , the change looks good. However I gather that there are test failures, so until those changes are committed we're not seeing the full impact. Also, if as you say, EXTRACT_DATE is obsolete, then we should remove it from the code base. I see that the Druid adapter makes use of EXTRACT_DATE, so we will need the Druid integration tests to pass before accepting this change.
          Hide
          Pavel Gubin Pavel Gubin added a comment - - edited

          Proposing changes according to this comment on CALCITE-1793

          Actually EXTRACT_DATE internal function can be removed then. The logic moved to RexImpTable.ExtractImplementor.

          Show
          Pavel Gubin Pavel Gubin added a comment - - edited Proposing changes according to this comment on CALCITE-1793 Actually EXTRACT_DATE internal function can be removed then. The logic moved to RexImpTable.ExtractImplementor.

            People

            • Assignee:
              julianhyde Julian Hyde
              Reporter:
              Pavel Gubin Pavel Gubin
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development