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

          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.
          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.
          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.
          Hide
          Kathey Marsden added a comment -

          Reopen for backport

          Show
          Kathey Marsden added a comment - Reopen for backport
          Hide
          Nirmal Fernando added a comment -

          Resolving this issue!

          Show
          Nirmal Fernando added a comment - Resolving this issue!
          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.
          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
          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 -

          > 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();
          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
          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
          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,

          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.
          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().
          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.

            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