Thread: getTiIme/Timestamp with TimeZone inconsistency

Looking at getTime and getTimestamp in TimestampUtils as used by the
getTime(int, Calendar) and getTimestamp(int, Calendar) functions in
AbstractJDBC2ResultSet there seems to be an inconsistency:

The API docs for these state "This method uses the given calendar to
construct an appropriate millisecond value for the time if the
underlying database does not store timezone information. "

and in TimestampUtils .getTimestamp if the server returns a timezone, it
is used instead of the supplied calendar.

However in TimestampUtils.getTime, this is initially done, but then
reversed further down and the supplied timezone is used as follows:

        if (ts.hasDate) {
            // Rotate it into the requested timezone before we zero out
the date
            .....
            cal.setTime(new Date(useCal.getTime().getTime()));
            useCal = cal;
        }

is there a reason for this as it seems inconsistent with getTimestamp
and the api docs?

Removing this makes the unit test TimezoneTest.testTime more consistent
(the first group of tests all return the same value - instead of the
last one being negative)


I ask as i'm playing with the adding binary wire transfer and Mikko
Tiihonen original patch omitted time/dates as he reported they failed
the unit tests.


Thanks

John Lister wrote:

> However in TimestampUtils.getTime, this is initially done, but then
> reversed further down and the supplied timezone is used as follows:
>
>        if (ts.hasDate) {
>            // Rotate it into the requested timezone before we zero out
> the date
>            .....
>            cal.setTime(new Date(useCal.getTime().getTime()));
>            useCal = cal;
>        }
>
> is there a reason for this as it seems inconsistent with getTimestamp
> and the api docs?

I believe this is necessary so that the Time type's invariants (that
day/month/year is Jan 1 1970 in the specified timezone) are satisfied.
The driver extends the interpretation of getTime(int,Calender) to mean
that the returned Time object will be valid for the specified Calendar,
regardless of whether a timezone was present in the underlying DB or not.

The underlying timezone, if present, is still used when constructing a
milliseconds value, but the driver then rotates that resulting value
into the target timezone. If we don't do that then you get the strange
result that retrieving a time "01:35:42" while providing a Calendar will
result in a Time object that does not display as 01:35:42 if you format
it with that same Calendar. (I hesitate to say whether this is right or
wrong, because as usual JDBC is woefully underspecified in this area,
but it's certainly confusing at a minimum)

-O

Oliver Jowett wrote:
> John Lister wrote:
>
>
>> However in TimestampUtils.getTime, this is initially done, but then
>> reversed further down and the supplied timezone is used as follows:
>>
>>        if (ts.hasDate) {
>>            // Rotate it into the requested timezone before we zero out
>> the date
>>            .....
>>            cal.setTime(new Date(useCal.getTime().getTime()));
>>            useCal = cal;
>>        }
>>
>> is there a reason for this as it seems inconsistent with getTimestamp
>> and the api docs?
>>
>
> I believe this is necessary so that the Time type's invariants (that
> day/month/year is Jan 1 1970 in the specified timezone) are satisfied.
> The driver extends the interpretation of getTime(int,Calender) to mean
> that the returned Time object will be valid for the specified Calendar,
> regardless of whether a timezone was present in the underlying DB or not.
>
> The underlying timezone, if present, is still used when constructing a
> milliseconds value, but the driver then rotates that resulting value
> into the target timezone. If we don't do that then you get the strange
> result that retrieving a time "01:35:42" while providing a Calendar will
> result in a Time object that does not display as 01:35:42 if you format
> it with that same Calendar. (I hesitate to say whether this is right or
> wrong, because as usual JDBC is woefully underspecified in this area,
> but it's certainly confusing at a minimum)
>
> -O
>
>

I agree the JDBC spec is vague, but I read the spec such that the
supplied calendar is *only* used if the server doesn't support a
timezone. I think my main concern is that setTimestamp behaves
differently to setTime. I'm not sure which is correct (i'd tend to the
former) but i think they should behave the same...

I'm not sure what the correct behaviour should be if the server has a
timezone and you specify one to use. Hopefully the app writer would only
use this case for none timezone columns/results.... maybe too much to
ask for :)

JOHN

John Lister wrote:

> I agree the JDBC spec is vague, but I read the spec such that the
> supplied calendar is *only* used if the server doesn't support a
> timezone. I think my main concern is that setTimestamp behaves
> differently to setTime. I'm not sure which is correct (i'd tend to the
> former) but i think they should behave the same...

The problem is that there's a bit of a disconnect between the server's
idea of time / timestamp with or without timezone, and the Java
representations. java.sql.Time is just a hideous hack to begin with.
It's going to be at best a patch job getting them to fit together.

> I'm not sure what the correct behaviour should be if the server has a
> timezone and you specify one to use. Hopefully the app writer would only
> use this case for none timezone columns/results.... maybe too much to
> ask for :)

I'm reluctant to touch that date/time handling code without an actual
testcase showing a problem. It's quite fragile as it is, and small
changes can have unintended consequences :/

-O