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

Add missing support for datetime JDBC functions

    Details

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

      Description

      Calcite advertises support for all datetime functions but implementations are missing for some of them.

      Since Calcite already supports EXTRACT(timeUnit FROM datetime), support could be added for the following functions: YEAR, MONTH, WEEK, DAYOFYEAR, DAYOFMONTH, DAYOFWEEK, HOUR, MINUTE, SECOND.

      Some concrete implementation (like WEEK) would be missing, but if RexImpTable is updated to support it, then it would be fully wired.

        Issue Links

          Activity

          Hide
          laurentgo Laurent Goujon added a comment -
          Show
          laurentgo Laurent Goujon added a comment - Proposed patch: https://github.com/apache/calcite/pull/364
          Hide
          julianhyde Julian Hyde added a comment - - edited

          Nice work.

          Because it is not standard SQL, I'm not sure that YEAR(dt) should be available in the default dialect. (I do think that EXTRACT(YEAR FROM dt)) and {{ fn YEAR(dt) }} should be available in all dialects.) Maybe they can move into a table analogous to OracleSqlOperatorTable. What do you think?

          In the doc, the highest value for MINUTE and SECOND is 59, I believe.

          As in your previous patch, I think that rewriteCall doesn't add much and can go.

          Show
          julianhyde Julian Hyde added a comment - - edited Nice work. Because it is not standard SQL, I'm not sure that YEAR(dt) should be available in the default dialect. (I do think that EXTRACT(YEAR FROM dt)) and {{ fn YEAR(dt) }} should be available in all dialects.) Maybe they can move into a table analogous to OracleSqlOperatorTable. What do you think? In the doc, the highest value for MINUTE and SECOND is 59, I believe. As in your previous patch, I think that rewriteCall doesn't add much and can go.
          Hide
          julianhyde Julian Hyde added a comment -

          PostgreSQL's dow function returns 0 - 6 (0 = Sunday). Should we be following that spec for DAYOFWEEK? Does any database or standard have a conflicting specification?

          Show
          julianhyde Julian Hyde added a comment - PostgreSQL's dow function returns 0 - 6 (0 = Sunday). Should we be following that spec for DAYOFWEEK? Does any database or standard have a conflicting specification?
          Hide
          laurentgo Laurent Goujon added a comment -

          I don't have strong opinion regarding moving these over to a different table, but since QUARTER was already present, it seemed logical to put them there too (I'm still struggling about finding what's standard or not). I also saw these functions being implemented by several databases too (at least for the main ones: YEAR/MONTH/DAY/HOUR/MINUTE/SECOND).

          For the doc, I agree, range should be between 0 and 59, I'll fix it

          I also agree for rewriteCall, I will remove it then rebasing against master

          As of dow, I used the ODBC spec (since I'm focusing on the scalar function support), which is described here: https://msdn.microsoft.com/en-us/library/ms714639(v=vs.85).aspx. Looks like that's the convention used by several databases (mysql, sql server). Oracle and DB2 seems to no implement DOW as an argument to extract on the other hand.

          Show
          laurentgo Laurent Goujon added a comment - I don't have strong opinion regarding moving these over to a different table, but since QUARTER was already present, it seemed logical to put them there too (I'm still struggling about finding what's standard or not). I also saw these functions being implemented by several databases too (at least for the main ones: YEAR/MONTH/DAY/HOUR/MINUTE/SECOND). For the doc, I agree, range should be between 0 and 59, I'll fix it I also agree for rewriteCall, I will remove it then rebasing against master As of dow, I used the ODBC spec (since I'm focusing on the scalar function support), which is described here: https://msdn.microsoft.com/en-us/library/ms714639(v=vs.85).aspx . Looks like that's the convention used by several databases (mysql, sql server). Oracle and DB2 seems to no implement DOW as an argument to extract on the other hand.
          Hide
          laurentgo Laurent Goujon added a comment -

          I just updated the pull request to fix the documentation and remove the extra rewriteCall methods in SqlJdbcFunctionCall. Let me know about what to do regarding the operator table.

          Show
          laurentgo Laurent Goujon added a comment - I just updated the pull request to fix the documentation and remove the extra rewriteCall methods in SqlJdbcFunctionCall . Let me know about what to do regarding the operator table.
          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/94cb5778 ; thanks for the PR, Laurent Goujon ! I logged CALCITE-1613 as a follow-up.
          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:
              laurentgo Laurent Goujon
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development