JtdsPreparedStatement setTime setTimestamp Daylight DST Bug
Brought to you by:
ickzon
Menu
▾
▴
#371 JtdsPreparedStatement setTime setTimestamp Daylight DST Bug
When one uses either of these two methods to set a
date in the database and 1) the machine running the
application is set to a time zone that observes Daylight
Saving Time (DST) and 2) the Calendar object being
used as the arguement in this call is set to a time zone
that does not observe DST and 3) the real world time is
currently in DST then 4) the time will be set incorrectly
in the database.
The problem is that these two methods use the
java.util.TimeZone.getRawOffset() method to obtain the
offset that must be applied before being sent to the
database. This is sufficient when the machine running
the application observes DST and the Calendar being
argued is observant as well. If the Calendar, however, is
set to a non observant time zone like US/Arizona and
the time in the world is in DST then the time sent to the
database will be one hour off from US/Arizona.
One possible solution would be to use the
java.util.TimeZone.getOffset(long date) method. This
method, from the javadocs,
"Returns the offset of this time zone from UTC at the
specified date. If Daylight Saving Time is in effect at the
specified date, the offset value is adjusted with the
amount of daylight saving."
"This method returns a historically correct offset value if
an underlying TimeZone implementation subclass
supports historical Daylight Saving Time schedule and
GMT offset changes."
Goodluck and thank you for developing such an
excellent library.
Regards,
McKinley
Logged In: YES
user_id=84089
NOTE: I refactored common code out of the three-argument
versions of JtdsPreparedStatement.setDate(), setTime() and
setTimestamp() into a private adjustTimeByCalendar() method.
Logged In: YES
user_id=84089
I forgot to mention: I am looking into this bug, but I WILL
NOT commit a fix to CVS without writing tests and posting
patches first. No guarantee that I'll get through this
soon, though.
Logged In: YES
user_id=84089
Attaching a patch with a test class that changes the
behavior of JtdsPreparedStatement.adjustTimeByCalendar() to
behave as requested.
HOWEVER, I DO NOT KNOW IF THIS IS REALLY THE CORRECT
BEHAVIOR. I don't understand the "contract" as it is
documented for the methods in the java.sql.PreparedStatement
interface, so I'm not sure what the correct behavior is.
Someone else needs to read the contract for those three,
three-argument methds from java.sql.PreparedStatement.
Logged In: YES
user_id=345242
Similar things needs to be done for JtdsResultSet. I
patched our driver to work by using getOffset(long) instead
of getRawOffset()
Logged In: YES
user_id=1252914
Apparently this bug also affects getTimestamp(int,
Calendar), which presently ignores daylight savings time.
If anyone knows of a workaround, please pass it along! Thanks.
Logged In: YES
user_id=84089
Please see this forum topic for more details:
http://sourceforge.net/forum/forum.php?thread_id=1258129&forum_id=129584
Logged In: YES
user_id=1252914
I'm confused by ddkilzer's last comment... thread 1258129 is
about caching prepared statements; it doesn't regard
conversion between a time zone that observes daylight
savings time and one that does not observe. Help me out
here, what am I missing?
Anxiously awaiting a patch for this time zone bug. :-)
Logged In: YES
user_id=345242
There is already a patch attached for fixing time on the way
in, and I already said how to fix time on the way out --
just change JtdsResultSet to use getOffset(time) rather than
getRawOffset.
Revised date/time handling
Logged In: YES
user_id=641437
All,
I have attached a patch to this bug report that I believe
fixes jTDS for both setting and getting date/time values
with an alternative Calendar.
There is a test case included which demonstrates the fixes.
This patch does not involve directly adding or subtracting
any offsets (raw or otherwise).
The test case runs identically with jTDS, jConnect and the
MS JDBC driver.
NB. Please read the comments in the test case before running
it as I am in the UK and there are special issues here
connected with the date 1970-01-01 and day light saving. If
running the test elsewhere you may need to change some of
the asserts.
David or Alin, if you have time please take a look at this
fix and see if you agree with the approach.
Mike.
Logged In: YES
user_id=84089
Sorry, bhawkins1, I copied and pasted the wrong forum URL.
Here is the correct one:
setTimestamp(Timestamp, Calendar) Bug?
http://sourceforge.net/forum/forum.php?thread_id=1260179&forum_id=129584
Mike's patch plus fixes listed above
Logged In: YES
user_id=84089
Attaching a patch with Mike's changes, plus:
calNY object's internal date/time was never changed.
the original time in the target Calendar object.
to be relative to the current timezone that the test is
being run in.
Mike, were you intending to set a default timezone in the
TimeZoneTest.testTimeZone()? I ask because the try/finally
block really doesn't do much around the test now.
Logged In: YES
user_id=641437
David,
I must apologise for the poor test case. As you suspected, I
had intended to change the default time zone to standardise
the test so it would work anywhere and then restore it at
the end. In practice this was never going to work, as the
static cal object in the driver would have already been
initialised to the real default zone before executing the
test. I think what I should have done is forget about
testing the default zone case and just rely on supplying a
Calendar object in all tests.
Hopefully this patch will fix the problems that people are
reporting. It would be good to get a few confirmations
before closing the report.
Mike.
Logged In: YES
user_id=564978
Ok, the fix in in CVS. Great jog, guys! I will notify all
users that signalled issues with timezones and DST to test
and see if everything works fine for them.
Richard,
If you're around, please get the latest CVS code and see if
it works for you.
Dave,
It would be great in the future when submitting patches to
submit them as zip files containing all modified files
rather than diffs, for two reasons: first because it's
easier for me to review the changes while applying them and
second (and most important) because I haven't been able to
figure out how to apply a patch. :"> I know that's not
characteristic of a Linux user, but that's just me.
Thanks,
Alin.
Logged In: YES
user_id=564978
Richard,
Any news? Does it work as expected?
Alin.
Logged In: YES
user_id=564978
No one seems to be reporting DST related issues anymore,
closing tracker.
Alin.