Обсуждение: 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