Derby
  1. Derby
  2. DERBY-4625

TIMESTAMP function doesn't accept nanoseconds

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.5.3.0
    • Fix Version/s: 10.5.3.2, 10.6.2.4, 10.7.1.1
    • Component/s: SQL
    • Labels:
      None

      Description

      The TIMESTAMP function fails if the string argument specifies the number of nanoseconds. It works if the argument is limited to microsecond resolution.

      ij> values timestamp('2010-04-21 12:00:00.123456');
      1
      --------------------------
      2010-04-21 12:00:00.123456

      1 row selected
      ij> values timestamp('2010-04-21 12:00:00.123456789');
      ERROR 22008: '2010-04-21 12:00:00.123456789' is an invalid argument to the timestamp function.

      Since Derby (and JDBC) supports nanosecond resolution, the TIMESTAMP function should also support it.

      1. derby_4625-2.diff
        4 kB
        Nirmal Fernando
      2. derby-4625-1.diff
        2 kB
        Nirmal Fernando

        Issue Links

          Activity

          Knut Anders Hatlen created issue -
          Knut Anders Hatlen made changes -
          Field Original Value New Value
          Component/s SQL [ 11408 ]
          Knut Anders Hatlen made changes -
          Link This issue relates to DERBY-4626 [ DERBY-4626 ]
          Knut Anders Hatlen made changes -
          Link This issue is related to DERBY-4614 [ DERBY-4614 ]
          Knut Anders Hatlen made changes -
          Link This issue relates to DERBY-2602 [ DERBY-2602 ]
          Hide
          Thiwanka Somasiri added a comment -

          Hi Knut,
          What are the classes that are used to call the timestamp functions?I referred mechanisms in UnaryDateTimestampOperatorNode.java class to some extent.

          Thanks.

          Show
          Thiwanka Somasiri added a comment - Hi Knut, What are the classes that are used to call the timestamp functions?I referred mechanisms in UnaryDateTimestampOperatorNode.java class to some extent. Thanks.
          Knut Anders Hatlen made changes -
          Description The TIMESTAMP function fails if the string argument specifies the number of nanoseconds. It works if the argument is limited to nanosecond resolution.

          ij> values timestamp('2010-04-21 12:00:00.123456');
          1
          --------------------------
          2010-04-21 12:00:00.123456

          1 row selected
          ij> values timestamp('2010-04-21 12:00:00.123456789');
          ERROR 22008: '2010-04-21 12:00:00.123456789' is an invalid argument to the timestamp function.

          Since Derby (and JDBC) supports nanosecond resolution, the TIMESTAMP function should also support it.
          The TIMESTAMP function fails if the string argument specifies the number of nanoseconds. It works if the argument is limited to microsecond resolution.

          ij> values timestamp('2010-04-21 12:00:00.123456');
          1
          --------------------------
          2010-04-21 12:00:00.123456

          1 row selected
          ij> values timestamp('2010-04-21 12:00:00.123456789');
          ERROR 22008: '2010-04-21 12:00:00.123456789' is an invalid argument to the timestamp function.

          Since Derby (and JDBC) supports nanosecond resolution, the TIMESTAMP function should also support it.
          Hide
          Knut Anders Hatlen added a comment -

          Hi Thiwanka,

          At compile time, calls to the TIMESTAMP function are represented by the TimestampOperatorNode class. At run time, the parsing of the timestamp string is performed by SQLTimestamp.parseTimestamp().

          Show
          Knut Anders Hatlen added a comment - Hi Thiwanka, At compile time, calls to the TIMESTAMP function are represented by the TimestampOperatorNode class. At run time, the parsing of the timestamp string is performed by SQLTimestamp.parseTimestamp().
          Nirmal Fernando made changes -
          Assignee C.S. Nirmal J. Fernando [ nirmal ]
          Hide
          Nirmal Fernando added a comment -

          Hi,

          I'm attaching a patch which solves this issue.
          This will probably break some tests since some properties have changed.
          I'm running tests now, and will attach a patch which will address updates
          to those tests soon.

          Thanks.

          Show
          Nirmal Fernando added a comment - Hi, I'm attaching a patch which solves this issue. This will probably break some tests since some properties have changed. I'm running tests now, and will attach a patch which will address updates to those tests soon. Thanks.
          Nirmal Fernando made changes -
          Attachment derby-4625-1.diff [ 12453427 ]
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Nirmal.

          Perhaps the change in DataTypeUtilities is better to leave out for now to make the impact on the tests smaller. If I understand correctly, it doesn't seem to be strictly necessary for getting the TIMESTAMP function to accept nanoseconds, it helps IJ display the entire value. Maybe that change could be made as part of DERBY-4614 instead?

          The patch seems to make an unintended change to the license header in SQLTimestamp.java.

          The constant SQLTimestamp.FRACTION_TO_NANO is used in SQLChar.setValue(Timestamp,Calendar). I think changing it from 1000 to 1 may make the calculations in SQLChar become wrong. (If we fix DERBY-4626 that code will probably go away, though.)

          It would also be good to have a test case (for example in DateTimeTest) that verifies that the function now accepts nanosecond resolution.

          Show
          Knut Anders Hatlen added a comment - Thanks, Nirmal. Perhaps the change in DataTypeUtilities is better to leave out for now to make the impact on the tests smaller. If I understand correctly, it doesn't seem to be strictly necessary for getting the TIMESTAMP function to accept nanoseconds, it helps IJ display the entire value. Maybe that change could be made as part of DERBY-4614 instead? The patch seems to make an unintended change to the license header in SQLTimestamp.java. The constant SQLTimestamp.FRACTION_TO_NANO is used in SQLChar.setValue(Timestamp,Calendar). I think changing it from 1000 to 1 may make the calculations in SQLChar become wrong. (If we fix DERBY-4626 that code will probably go away, though.) It would also be good to have a test case (for example in DateTimeTest) that verifies that the function now accepts nanosecond resolution.
          Hide
          Nirmal Fernando added a comment -

          Hi Knut,

          Thanks for reviewing the patch.

          >The constant SQLTimestamp.FRACTION_TO_NANO is used in SQLChar.setValue(Timestamp,Calendar). I think changing it from 1000 to 1 may make the calculations in >SQLChar become wrong. (If we fix DERBY-4626 that code will probably go away, though.)

          Without changing 1000 to 1 we can't get the expected result. I am wondering whether there are test cases which is addressed this setValue(..) function. If so since I did not find any regression failures regarding that, can't we conclude that the calculations of SQLChar are still correct?

          Thanks.

          Show
          Nirmal Fernando added a comment - Hi Knut, Thanks for reviewing the patch. >The constant SQLTimestamp.FRACTION_TO_NANO is used in SQLChar.setValue(Timestamp,Calendar). I think changing it from 1000 to 1 may make the calculations in >SQLChar become wrong. (If we fix DERBY-4626 that code will probably go away, though.) Without changing 1000 to 1 we can't get the expected result. I am wondering whether there are test cases which is addressed this setValue(..) function. If so since I did not find any regression failures regarding that, can't we conclude that the calculations of SQLChar are still correct? Thanks.
          Hide
          Nirmal Fernando added a comment -

          Hi,

          I have attached a patch to DERBY-4614 which is intended to address this issue as well.
          Since both issues are co-related to some extent, it's difficult to provide a separate patch.

          Thanks.

          Show
          Nirmal Fernando added a comment - Hi, I have attached a patch to DERBY-4614 which is intended to address this issue as well. Since both issues are co-related to some extent, it's difficult to provide a separate patch. Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          > I am wondering whether there are test cases which is addressed this
          > setValue(..) function. If so since I did not find any regression
          > failures regarding that, can't we conclude that the calculations of
          > SQLChar are still correct?

          It seems that the calculation in SQLChar changed, but in a way that
          fixed DERBY-4626. So that's actually a good thing!

          However, the change also makes the code in SQLChar a bit
          confusing. Take for instance this fragment:

          int micros =
          (theValue.getNanos() + SQLTimestamp.FRACTION_TO_NANO/2) /
          SQLTimestamp.FRACTION_TO_NANO;

          After the patch, the micros variable will actually hold the nanosecond
          value, so we should change its name to nanos. Also, since the above
          code now transforms nanoseconds to nanoseconds (that is, it doesn't do
          anything), we should probably simplify it to:

          int nanos = theValue.getNanos();

          Show
          Knut Anders Hatlen added a comment - > I am wondering whether there are test cases which is addressed this > setValue(..) function. If so since I did not find any regression > failures regarding that, can't we conclude that the calculations of > SQLChar are still correct? It seems that the calculation in SQLChar changed, but in a way that fixed DERBY-4626 . So that's actually a good thing! However, the change also makes the code in SQLChar a bit confusing. Take for instance this fragment: int micros = (theValue.getNanos() + SQLTimestamp.FRACTION_TO_NANO/2) / SQLTimestamp.FRACTION_TO_NANO; After the patch, the micros variable will actually hold the nanosecond value, so we should change its name to nanos. Also, since the above code now transforms nanoseconds to nanoseconds (that is, it doesn't do anything), we should probably simplify it to: int nanos = theValue.getNanos();
          Nirmal Fernando made changes -
          Attachment derby_4625-2.diff [ 12454845 ]
          Hide
          Nirmal Fernando added a comment -

          Patch #2 is addressing Knut's latest comment!
          Hope this issue is solved now!

          Thanks.

          Show
          Nirmal Fernando added a comment - Patch #2 is addressing Knut's latest comment! Hope this issue is solved now! Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Nirmal!

          The patch looks good to me. The new test case in DateTimeTest was commented out. I've commented it back in and started the regression tests suites. I'll commit the fix if the tests pass.

          Show
          Knut Anders Hatlen added a comment - Thanks, Nirmal! The patch looks good to me. The new test case in DateTimeTest was commented out. I've commented it back in and started the regression tests suites. I'll commit the fix if the tests pass.
          Hide
          Knut Anders Hatlen added a comment -

          All the regression tests ran cleanly.
          Committed revision 999479.

          Show
          Knut Anders Hatlen added a comment - All the regression tests ran cleanly. Committed revision 999479.
          Knut Anders Hatlen made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Fix Version/s 10.7.0.0 [ 12314971 ]
          Resolution Fixed [ 1 ]
          Rick Hillegas made changes -
          Fix Version/s 10.7.1.1 [ 12315564 ]
          Fix Version/s 10.7.1.0 [ 12314971 ]
          Nirmal Fernando made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Hide
          Nirmal Fernando added a comment -

          Resolving this issue!

          Show
          Nirmal Fernando added a comment - Resolving this issue!
          Nirmal Fernando made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Issue & fix info [Repro attached]
          Resolution Fixed [ 1 ]
          Kathey Marsden made changes -
          Link This issue is required by DERBY-4994 [ DERBY-4994 ]
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Kathey Marsden added a comment -

          Reopen for backport

          Show
          Kathey Marsden added a comment - Reopen for backport
          Kathey Marsden made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Hide
          Mike Matrigali added a comment -

          temp taking ownership to backport to 10.6 and 10.5.

          Show
          Mike Matrigali added a comment - temp taking ownership to backport to 10.6 and 10.5.
          Mike Matrigali made changes -
          Assignee C.S. Nirmal J. Fernando [ nirmal ] Mike Matrigali [ mikem ]
          Hide
          Mike Matrigali added a comment -

          backported fix to 10.5 and 10.6. resolving and resetting original owner.

          Show
          Mike Matrigali added a comment - backported fix to 10.5 and 10.6. resolving and resetting original owner.
          Mike Matrigali made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Assignee Mike Matrigali [ mikem ] C.S. Nirmal J. Fernando [ nirmal ]
          Fix Version/s 10.6.2.1 [ 12315343 ]
          Fix Version/s 10.5.3.0 [ 12314117 ]
          Resolution Fixed [ 1 ]
          Hide
          Mike Matrigali added a comment -

          fixing released version for the backports. mistakenly picked released versions rather than unreleased versions.

          Show
          Mike Matrigali added a comment - fixing released version for the backports. mistakenly picked released versions rather than unreleased versions.
          Mike Matrigali made changes -
          Fix Version/s 10.5.3.2 [ 12315436 ]
          Fix Version/s 10.6.2.3 [ 12315434 ]
          Fix Version/s 10.5.3.0 [ 12314117 ]
          Fix Version/s 10.6.2.1 [ 12315343 ]
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Workflow jira [ 12508972 ] Default workflow, editable Closed status [ 12801116 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Closed Closed
          153d 5h 9m 1 Knut Anders Hatlen 21/Sep/10 17:07
          Closed Closed Reopened Reopened
          133d 2h 50m 2 Kathey Marsden 02/Feb/11 23:43
          Reopened Reopened Resolved Resolved
          13d 24m 2 Mike Matrigali 16/Feb/11 00:05
          Resolved Resolved Closed Closed
          135d 13h 58m 2 Knut Anders Hatlen 30/Jun/11 10:21

            People

            • Assignee:
              Nirmal Fernando
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development