[
https://issues.apache.org/jira/browse/DERBY-1816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12494182
]
Daniel John Debrunner commented on DERBY-1816:
----------------------------------------------
Looking at the network client's implementation of getTimeStamp() with a
Calendar I have to say it seems inefficient, confusing and wrong. :-)
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).
> Client's ResultSet.getTime() on a SQL TIMESTAMP column loses the sub-second
> resolution and always has a milli-second value of zero.
> -----------------------------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-1816
> URL: https://issues.apache.org/jira/browse/DERBY-1816
> Project: Derby
> Issue Type: Bug
> Components: JDBC, Network Client
> Affects Versions: 10.1.1.0, 10.1.2.1, 10.1.3.1, 10.2.1.6, 10.3.0.0
> Reporter: Daniel John Debrunner
> Assigned To: A B
> Priority: Minor
> Attachments: d1816_recycleCleanup_v1.patch
>
>
> In embedded the java.sql.Time object returned from ResultSet.getTime() for a
> SQL TIMESTAMP object has its millisecond value for the time portion equal to
> that for the java.sql.Timestamp value.
> In client the millisecond time value for such a value is always set to zero.
> Note a Derby SQL TIME value has by definition resolution of only a second so
> its millisecond value is always zero,
> but java.sql.Time is not a direct mapping to the SQL Type, it's a JDBC type,
> so when converting from a SQL TIMESTAMP
> it should retain the precision.
> The new test lang.TimeHandlingTest has this assert code that shows the
> problem, one of its calls will be commented out
> with a comment with this bug number.
> private void assertTimeEqual(Time tv, Timestamp tsv)
> {
> cal.clear();
> cal.setTime(tv);
>
> int hour = cal.get(Calendar.HOUR_OF_DAY);
> int min = cal.get(Calendar.MINUTE);
> int sec = cal.get(Calendar.SECOND);
> int ms = cal.get(Calendar.MILLISECOND);
>
> // Check the time portion is set to the same as tv
> cal.clear();
> cal.setTime(tsv);
> assertEquals(hour, cal.get(Calendar.HOUR_OF_DAY));
> assertEquals(min, cal.get(Calendar.MINUTE));
> assertEquals(sec, cal.get(Calendar.SECOND));
> assertEquals(ms, cal.get(Calendar.MILLISECOND)); <<<<<<<<<<<<<
> FAILS HERE
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
