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

Calcite timestamp literals cannot express precision above millisecond, TIMESTAMP(3)

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: avatica-1.10.0, 1.13.0
    • Component/s: None
    • Labels:
      None

      Description

      RexBuilder.makeTimestampLiteral accepts the TS as a Java Calendar instance. Calendar type has only ms precision, thus types like TIMESTAMP(9) cannot be represented.
      This results in incorrect results in Hive due to constant reduction:

      hive> explain select c17 from testjdbcdriverdatatypetbl where c17='2012-04-22 09:00:00.123456789';
      OK
      Plan optimized by CBO.
      
      Stage-0
        Fetch Operator
          limit:-1
          Select Operator [SEL_2]
            Output:["_col0"]
            Filter Operator [FIL_4]
              predicate:(c17 = 2012-04-22 09:00:00.123)
              TableScan [TS_0]
                Output:["c17"]
      
      hive> select c17 from testjdbcdriverdatatypetbl where c17='2012-04-22 09:00:00.123456789';
      OK
      Time taken: 0.687 seconds
      
      hive> set hive.cbo.enable=false;
      hive> explain select c17 from testjdbcdriverdatatypetbl where c17='2012-04-22 09:00:00.123456789';
      OK
      Stage-0
        Fetch Operator
          limit:-1
          Select Operator [SEL_2]
            Output:["_col0"]
            Filter Operator [FIL_4]
              predicate:(c17 = '2012-04-22 09:00:00.123456789')
              TableScan [TS_0]
                Output:["c17"]
      
      hive> select c17 from testjdbcdriverdatatypetbl where c17='2012-04-22 09:00:00.123456789';
      OK
      2012-04-22 09:00:00.123
      

      Note how with CBO enabled the qualified row is missed. The plan shows that the constant was truncated to ms.

        Issue Links

          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).
          Show
          julianhyde Julian Hyde added a comment - Fixed in Calcite in http://git-wip-us.apache.org/repos/asf/calcite/commit/205af813 .
          Hide
          julianhyde Julian Hyde added a comment -

          Fixed in Avatica in http://git-wip-us.apache.org/repos/asf/calcite-avatica/commit/3c40e0db; Calcite work will follow after avatica-1.10.

          Show
          julianhyde Julian Hyde added a comment - Fixed in Avatica in http://git-wip-us.apache.org/repos/asf/calcite-avatica/commit/3c40e0db ; Calcite work will follow after avatica-1.10.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/calcite-avatica/pull/9

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/calcite-avatica/pull/9
          Hide
          julianhyde Julian Hyde added a comment -

          Yes it would be good to discuss dropping JDK 7. Can you start that thread?

          Whatever the outcome of that discussion, I am happy with the decision to use TimestampString etc. for literals in Calcite. They are simple, immutable, support unlimited precision, and it's easy to convert them to and from Calendar, Joda-Time, or whatever.

          Show
          julianhyde Julian Hyde added a comment - Yes it would be good to discuss dropping JDK 7. Can you start that thread? Whatever the outcome of that discussion, I am happy with the decision to use TimestampString etc. for literals in Calcite. They are simple, immutable, support unlimited precision, and it's easy to convert them to and from Calendar, Joda-Time, or whatever.
          Hide
          ashutoshc Ashutosh Chauhan added a comment -

          May be its time to drop support for jdk7. jdk8 is gonna be end of life in another 3 months. Since, Calcite is used more as 'platform' on which other products build on and not many usages are like 'end-user' scenario. Does supporting a version which was EOL 2 years ago worth the confusion in api?
          Worth a discussion on dev list?

          Show
          ashutoshc Ashutosh Chauhan added a comment - May be its time to drop support for jdk7. jdk8 is gonna be end of life in another 3 months. Since, Calcite is used more as 'platform' on which other products build on and not many usages are like 'end-user' scenario. Does supporting a version which was EOL 2 years ago worth the confusion in api? Worth a discussion on dev list?
          Hide
          julianhyde Julian Hyde added a comment -

          Calcite is still on jdk 1.7 - 9. For literal, we have dumped Calendar, and neither joda nor java.time were suitable. I think you'll find TimestampString simple and effective. It just contains a formatted string. It is immutable, and can represent as many fractional digits as you want. It's easy to convert TimestampString to or from milliseconds or nanoseconds.

          Show
          julianhyde Julian Hyde added a comment - Calcite is still on jdk 1.7 - 9. For literal, we have dumped Calendar, and neither joda nor java.time were suitable. I think you'll find TimestampString simple and effective. It just contains a formatted string. It is immutable, and can represent as many fractional digits as you want. It's easy to convert TimestampString to or from milliseconds or nanoseconds.
          Hide
          ashutoshc Ashutosh Chauhan added a comment -

          Quick note: FWIW Hive has moved to jdk8 only and we plan to move our timestamps off of java.sql.Timestamp and on to java.time

          Show
          ashutoshc Ashutosh Chauhan added a comment - Quick note: FWIW Hive has moved to jdk8 only and we plan to move our timestamps off of java.sql.Timestamp and on to java.time
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user julianhyde opened a pull request:

          https://github.com/apache/calcite-avatica/pull/9

          CALCITE-1690 Timestamp literals cannot express precision above millisecond

          Required change for Calcite issue https://issues.apache.org/jira/browse/CALCITE-1690.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/julianhyde/calcite-avatica 1690-date-time-literal

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/calcite-avatica/pull/9.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #9


          commit cc7a80a330880a30c4f222fee17d24c734daa26a
          Author: Julian Hyde <jhyde@apache.org>
          Date: 2017-05-03T02:34:40Z

          CALCITE-1690 Timestamp literals cannot express precision above millisecond


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user julianhyde opened a pull request: https://github.com/apache/calcite-avatica/pull/9 CALCITE-1690 Timestamp literals cannot express precision above millisecond Required change for Calcite issue https://issues.apache.org/jira/browse/CALCITE-1690 . You can merge this pull request into a Git repository by running: $ git pull https://github.com/julianhyde/calcite-avatica 1690-date-time-literal Alternatively you can review and apply these changes as the patch at: https://github.com/apache/calcite-avatica/pull/9.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #9 commit cc7a80a330880a30c4f222fee17d24c734daa26a Author: Julian Hyde <jhyde@apache.org> Date: 2017-05-03T02:34:40Z CALCITE-1690 Timestamp literals cannot express precision above millisecond
          Hide
          julianhyde Julian Hyde added a comment -

          Here's how to create a high-precision timestamp literal:

          final RexBuilder b;
          // Your type system must support enough digits;
          // otherwise the literal will be silently rounded on creation
          assert b.getTypeFactory().getTypeSystem()
              .getMaxPrecision(SqlTypeName.TIMESTAMP) >= 9;
          // July 21st 1969 02:56:40 and 12,345,678 nanoseconds.
          // The leading "0" is important; without it you'd have 123,456,780 nanoseconds.
          final TimestampString ts = new TimestampString(1969, 7, 21, 2, 56, 40)
            .withFraction("012345678");
          RexLiteral literal = b.makeTimestampLiteral(ts, 9);
          
          // Or, if you don't need to go further than nanoseconds,
          final TimestampString ts2 = new TimestampString(1969, 7, 21, 2, 56, 40)
            .withNanos(12345678);
          
          // We recommend using RexLiteral.getValueAs(Class) to retrieve the value.
          // The behavior of getValue, getValue2, getValue3 is unchanged.
          Calendar cal = literal.getValueAs(Calendar.class); // only contains milliseconds
          Comparable c = literal.getValue();
          assert c.equals(cal);
          Long l = literal.getValueAs(Long.class);  // only contains milliseconds
          TimestampString ts3 = literal.getValueAs(TimestampString.class); // contains all digits
          assert ts3.equals(ts);
          assert ts3.equals(ts2);
          

          There is a patch for review at https://github.com/julianhyde/calcite/tree/1690-date-time-literal. It depends on changes to avatica, which can be found at https://github.com/julianhyde/calcite-avatica/tree/1690-date-time-literal. You will need to "mvn install" avatica, and this fix will require an avatica-1.10 release.

          Ashutosh Chauhan, Remus Rusanu, Can you please review?

          Show
          julianhyde Julian Hyde added a comment - Here's how to create a high-precision timestamp literal: final RexBuilder b; // Your type system must support enough digits; // otherwise the literal will be silently rounded on creation assert b.getTypeFactory().getTypeSystem() .getMaxPrecision(SqlTypeName.TIMESTAMP) >= 9; // July 21st 1969 02:56:40 and 12,345,678 nanoseconds. // The leading "0" is important; without it you'd have 123,456,780 nanoseconds. final TimestampString ts = new TimestampString(1969, 7, 21, 2, 56, 40) .withFraction( "012345678" ); RexLiteral literal = b.makeTimestampLiteral(ts, 9); // Or, if you don't need to go further than nanoseconds, final TimestampString ts2 = new TimestampString(1969, 7, 21, 2, 56, 40) .withNanos(12345678); // We recommend using RexLiteral.getValueAs( Class ) to retrieve the value. // The behavior of getValue, getValue2, getValue3 is unchanged. Calendar cal = literal.getValueAs(Calendar.class); // only contains milliseconds Comparable c = literal.getValue(); assert c.equals(cal); Long l = literal.getValueAs( Long .class); // only contains milliseconds TimestampString ts3 = literal.getValueAs(TimestampString.class); // contains all digits assert ts3.equals(ts); assert ts3.equals(ts2); There is a patch for review at https://github.com/julianhyde/calcite/tree/1690-date-time-literal . It depends on changes to avatica, which can be found at https://github.com/julianhyde/calcite-avatica/tree/1690-date-time-literal . You will need to "mvn install" avatica, and this fix will require an avatica-1.10 release. Ashutosh Chauhan , Remus Rusanu , Can you please review?
          Hide
          julianhyde Julian Hyde added a comment -

          Turns out that Joda-Time, while similar to java.time, doesn't support nanoseconds. And java.time is not available on JDK 1.7.

          So, I plan to create classes DateString, TimeString and TimestampString which are immutable wrappers around strings such as "2017-04-28", "12:34:56", "2017-04-29 12:34:56.78901234567". They'll have operations to create from (year, month, date), (hour, minute, second), as many fractional digits as you want (that's the great thing about strings), and simple conversion to and from millis-since-epoch and days-since-epoch.

          Show
          julianhyde Julian Hyde added a comment - Turns out that Joda-Time, while similar to java.time, doesn't support nanoseconds. And java.time is not available on JDK 1.7. So, I plan to create classes DateString , TimeString and TimestampString which are immutable wrappers around strings such as "2017-04-28", "12:34:56", "2017-04-29 12:34:56.78901234567". They'll have operations to create from (year, month, date), (hour, minute, second), as many fractional digits as you want (that's the great thing about strings), and simple conversion to and from millis-since-epoch and days-since-epoch.
          Hide
          ashutoshc Ashutosh Chauhan added a comment -

          1. Hive has nanosecond precision. So, its sufficient for now. Forseeable future, it does seem unlikely that higher precision will be needed.

          2. Your proposal sounds good to me. Although you didn't mention but I assume you also plan to add RexLiteral constructor to accept LocalDateTime. So, someone can use LocalDateTime all through which never gets converted to Calendar.

          Show
          ashutoshc Ashutosh Chauhan added a comment - 1. Hive has nanosecond precision. So, its sufficient for now. Forseeable future, it does seem unlikely that higher precision will be needed. 2. Your proposal sounds good to me. Although you didn't mention but I assume you also plan to add RexLiteral constructor to accept LocalDateTime. So, someone can use LocalDateTime all through which never gets converted to Calendar.
          Hide
          julianhyde Julian Hyde added a comment -

          The SQL standard puts no upper bound on precision. In SQL-2014 6.1 <data type>, Syntax Rules:

          36) The maximum value of <time precision> and the maximum value of <timestamp precision> shall be the same implementation-defined value that is not less than 6. The values of <time precision> and <timestamp precision> shall not be greater than that maximum value.

          Show
          julianhyde Julian Hyde added a comment - The SQL standard puts no upper bound on precision. In SQL-2014 6.1 <data type>, Syntax Rules: 36) The maximum value of <time precision> and the maximum value of <timestamp precision> shall be the same implementation-defined value that is not less than 6. The values of <time precision> and <timestamp precision> shall not be greater than that maximum value.
          Hide
          julianhyde Julian Hyde added a comment -

          Calcite already uses Joda-Time, and its LocalDateTime type has a precision down to nanoseconds. Two questions:

          1. Are nanoseconds sufficient (now and for the foreseeable future)?

          2. If we switch the type of the value inside RexLiteral from Calendar (with UTC time zone) to LocalDateTime, is this going to break clients that are used to a Calendar?

          To solve 2, I propose that:

          • RexLiteral's constructor continues to accept Calendar (but internally converts to LocalDateTime, which is what gets stored);
          • RexLiteral.value(RexNode) and RexLiteral.getValue() continue to return a Calendar (creating one on the fly); and
          • we add a new method <T> T RexLiteral.value(Class<T>) so that literal.value(LocalDateTime.class) will return the LocalDateTime.
          Show
          julianhyde Julian Hyde added a comment - Calcite already uses Joda-Time, and its LocalDateTime type has a precision down to nanoseconds. Two questions: 1. Are nanoseconds sufficient (now and for the foreseeable future)? 2. If we switch the type of the value inside RexLiteral from Calendar (with UTC time zone) to LocalDateTime , is this going to break clients that are used to a Calendar ? To solve 2, I propose that: RexLiteral 's constructor continues to accept Calendar (but internally converts to LocalDateTime , which is what gets stored); RexLiteral.value(RexNode) and RexLiteral.getValue() continue to return a Calendar (creating one on the fly); and we add a new method <T> T RexLiteral.value(Class<T>) so that literal.value(LocalDateTime.class) will return the LocalDateTime .
          Hide
          rusanu Remus Rusanu added a comment -

          We can work around this issue, from Hive side, by preventing constant reduciton of TIMESTMAP values that have sub-millisecond precision.

          Show
          rusanu Remus Rusanu added a comment - We can work around this issue, from Hive side, by preventing constant reduciton of TIMESTMAP values that have sub-millisecond precision.
          Hide
          rusanu Remus Rusanu added a comment -

          Any opinion on how to address this? Supporting ANSI TIMESTMAP(9) would require that Calcite changes the internal representation. I'm guessing java.time.* types are not an option since it would make JDK 1.8 a hard requirement. My frst instinct is to give java.sql.Timestamp a try as internal Calcite representation for TIMESTMAP.

          Show
          rusanu Remus Rusanu added a comment - Any opinion on how to address this? Supporting ANSI TIMESTMAP(9) would require that Calcite changes the internal representation. I'm guessing java.time.* types are not an option since it would make JDK 1.8 a hard requirement. My frst instinct is to give java.sql.Timestamp a try as internal Calcite representation for TIMESTMAP.

            People

            • Assignee:
              julianhyde Julian Hyde
              Reporter:
              rusanu Remus Rusanu
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development