[jira] Created: (DERBY-2690) Clean up client's implementation of getTimestamp() with a Calendar argument.

["A B (JIRA)" <ji...@apache.org>]

Clean up client's implementation of getTimestamp() with a Calendar argument.
----------------------------------------------------------------------------

                 Key: DERBY-2690
                 URL: https://issues.apache.org/jira/browse/DERBY-2690
             Project: Derby
          Issue Type: Improvement
          Components: Network Client
    Affects Versions: 10.3.0.0
            Reporter: A B
            Priority: Minor


[ As pasted from a comment from Dan re: DERBY-1816. ]

This is looking at lines 1020-1031 in client.am.ResultSet.java (which isn't modified by the patch so it's existing code)

Inefficient because it creates two new Calendar objects for every call,that will cause a heavy gc overload and cpu overhead.

Confusing because it uses two Calendar objects (when one will do) and just does strange things:
    - The value from the column is set in the targetCalendar but then never used
    - The value from the column is set in the defaultCalendar but then never used
    - The timezeone offset is calculated using the two calendar objects, and then applied to the long millisecond value obtained from the
       java.sql.Timestamp value, but this is what Calendar objects do, so why is this being done explicitly?
    - no code comments

Wrong because:
    - the passed in calendar is not used to perform the conversion from SQL TIMESTAMP value to java.sql.Timestamp. The passed in Calendar is only used to determine the timezone of the value to be returned (line 1021). However a Calendar object has more meaning than just a time zone, it has behaviour because a Calendar object is an abstract class and thus can have multiple different implementations. The client code is assuming that the Calendar object returned from the static methods Calendar.getInstance() match the Calendar object passed in, this may not be true. If the application passes in a Jewish Calendar implementation for example then most likely this client code will be using a GregorianCalendar to incorrectly perform calculations.

  - looking beyond these lines of code, I think the complete conversion is incorrect, even if in this method the passed in Calendar was used. Before these lines of code the value is obtained using the getTimestamp() with no Calendar object, this uses a GregorianCalendar() to convert from the over-the-wire value to a java.sql Timestamp. Thus the conversion would be (if the user supplied Calendar object was used):

   YYYY-MM-DD:hh:mm:ss.ffff >> GregorianCalendar >> long milli-seconds >> java.sql.Timestamp >> long milli-seconds >> user supplied Calendar object

I'm think that if the user supplied Calendar object was not an instance of GregorianCalendar then there's a significant chance the wrong result would be returned.

What is required is the same approach as embedded, which is to always perform
   YYYY-MM-DD:hh:mm:ss.ffff >> Calendar >> long milli-seconds >> java.sql.Timestamp

where the Calendar is either the user-supplied one or a builtin one, so that the calling order is the other way around, getTimestamp(int) should be calling getTimestamp(int, Calendar).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.