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.
