db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "A B (JIRA)" <j...@apache.org>
Subject [jira] Created: (DERBY-2690) Clean up client's implementation of getTimestamp() with a Calendar argument.
Date Thu, 24 May 2007 15:47:16 GMT
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:
            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.

View raw message