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

            is depended upon by

            Bug - A problem which impairs or prevents the functions of the product. CALCITE-1613 Implement EXTRACT for time unit DOW, DOY; and fix CENTURY

            • Major - Major loss of function.
            • Closed

              Activity

              Ascending order - Click to sort in descending order
              Hide
              Permalink
              laurentgo Laurent Goujon added a comment -

              Proposed patch: https://github.com/apache/calcite/pull/364

              Show
              laurentgo Laurent Goujon added a comment - Proposed patch: https://github.com/apache/calcite/pull/364
              Hide
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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.

              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
              Permalink
              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