Details
Description
Castor has a method for forcing what timezone to read dates with. (See http://www.castor.org/types.html#SQL-Dates-and-Default-Timezone). The problem is that this setting has not effect on what you write to the database. So you don't have round-trip integrity of date, time and timestamp values.
For example, if I set the timezone to be 'UTC' in the castor.properties. When I write a date to the database it will write it to the database as 'PDT' (my local timezone) cause this is what the JDBC driver defaults too. But when I read it back it will convert it to 'UTC' because castor is is only converting on reads. So my dates will be 8 hours off of what they should be. Anyway, not the clearest explanation...sorry. I've attached a modified SQLTypeInfos.java file that should address the issue.
-
- patch.c1276.20051202.txt
- 02/Dec/05 3:13 AM
- 1 kB
- Werner Guttmann
-
- patch.C1276.20051218.txt
- 17/Dec/05 2:15 PM
- 7 kB
- Werner Guttmann
-
- patch.C1276.20051218-002.txt
- 17/Dec/05 3:31 PM
- 34 kB
- Werner Guttmann
-
- patch.C1276.20051219.test.txt
- 18/Dec/05 12:40 PM
- 9 kB
- Werner Guttmann
-
- patch-C1276-20070130.txt
- 30/Jan/07 4:32 PM
- 11 kB
- Ralf Joachim
-
- patch-C1276-20070205.txt
- 05/Feb/07 3:48 PM
- 16 kB
- Ralf Joachim
-
- patch-C1276-20070307.txt
- 06/Mar/07 5:44 PM
- 33 kB
- Ralf Joachim
-
- SQLTypeInfos.java
- 01/Dec/05 5:32 PM
- 13 kB
- Brian Schlining
-
- TestTemplate.java
- 31/Jan/07 4:51 PM
- 10 kB
- Brian Schlining
Issue Links
- is depended upon by
-
CASTOR-1285
Complete support for Calendar during reads/writes to apply to various type conversions
-
Activity
Sorry about not submitting just a patch....future contributions will be in patch form.
Brian, it looks like your patch is a valid one (after inserting a few break; statements throughout your additions ..
), but unfortunately it falls short from providing a complete solution, as with Castor a java.util.Date property of a Java class about to be persisted can be mapped to numeric and string columns as well. In my view, one needs to add code to SQLEngine.toSql() to set the Calendar instance on the java.util.Date object instance before it's being converted subsequently.
Updated patch, incl. a few new break; statements. In addition, this patch carries a TODO statement in SQLEngine.toSql() to get an idea what I am talking about above.
Werner, you are right that Brian's patch to support timezone at store do not cover Date's that are mapped to char, integer or numeric. If you take at the timezone support when loading these dates the mappings to char, integer and numeric are also ignored. I suggest to commit your original patch and don't add this support yet at least not in SQLEngine. If you really want to support these other mappings I think the much better place are SQLTypeConverters.
Okay, fine by me, I'll commit the original patch as it is, and add a new issue to cover extended support during conversions. It's funny you are mentioning SQLTypeConverters, as I have been thinking about adding code in some of those convertors. 'Problem' is that you'd have to add the same piece of code over and over again.
Final patch, as discussed above. I still have to add a CTF test, though.
Started to sketch up a small test in src/bugs, but to my surprise it does not fail. I thought setting different timezones for updates/creates and reads should make it possible to test this functionality, but to my surprise this is not the case.
Unless somebody comes up with a bit more structured idea how to test this, I am going to pull this from 1.0M1 and assign this to 1.0 (final).
Hmmm, somehow my watch on this issue got turned off, so I missed most of the traffic. I'll take a look into this later this week.
For a workaround to this issue, I have been setting the JVM's timezone to UTC on startup. [Just FYI, for this to work you have to set the timezone on the command line (using -Duser.timzone=UTC) or in your program before you initialize any Date, Calendar or logging objects (using System.setProperty("user.timezone", "UTC");]
Recently, I've begun deploying my apps using Java Web Start (JWS). Unfortunately, due to security restrictions in JWS, I can no longer set the timezone of the JVM. (see http://forum.java.sun.com/thread.jspa?forumID=38&threadID=735277). So I'm under some pressure to find a resolution to this timezone issue.
If my reading of the thread is on track, it looks like some work was done on this but it's not clear to me if the patches can be applied to the SVN trunk or even if the patches represent the solution that you intend to implement. If you can point me to the recommended solution (modify SQLEngine or SQLTypeConvertors?) I'm happy to take a stab at it and write up a few tests.
Brian, looking at the patch it seams to me that it should cover date values of your domain objects written to TIMESTAMP, DATE and TIME columns of your database. Date values written to LONG or CHAR columns are not covered yet. As far as I understand Werner's latest comments he had problems to write a test case to reproduce the problem and that it is fixed by the patch.
I will update the patch to current SVN and hope this will help you to test the patch and provide us a test case to reproduce thze problem and verify the solution.
Updated patch against current SVN. Having said that I only updated the patch but did not test anything yet.
Thanks Ralf,
I'm checking out SVN trunk right now and will run some tests in the application that depends on this functionality in Castor (http://vars.sourceforge.net). Once I've verified that it works with my application, I'll write a few test cases.
I found Werner's test case in castor/src/bugs/jdo/bug1276. Rather than wrestle with getting setup to run that test, I created a test using my existing application to check 'datetime'/'timestamp' fields. The test I ran is at http://vars.svn.sourceforge.net/viewvc/vars/trunk/vars/src/test/java/org/mbari/jdo/castor/TestDateStorage.java?view=markup. The patch works, at least for my cases.
I added one more test method to src/bugs/jdo/bug1276/TestTemplate.java and attached the new file. Sorry I didn't generate a unified patch; it's tough to generate a patch for a patch that hasn't been applied to SVN yet
. To commit it to SVN you have to apply your patch (patch-C1276-20070130.txt) then copy the new TestTemplate.java file to src/bugs/jdo/bug1276/. Unfortunately, the added test method only checks 'timestamp' SQL data types. I'm under a dealine crunch so I'll try to get tests for 'date' and 'time' SQL data types done 'REAL SOON'.
Thanks, Brian. And unless you need that fix for your deadline(s), take yourself time.
As you may be aware, with the current builds of Castor, setting the timezone will currupt dates, times, and timestamps. For example, if you set the timezone in castor.properties, then calling something like
// Set timezone as UTC in castor.properties. Local timezone is PDT
Date d = entity.getTimestamp();
entity.setTimestamp(d);
// Start new database transaction
d = entity.getTimestamp();
entity.setTimestamp(d);
will cause the date to 'walk' because Castor is reading from the date from the database as UTC but writting it as PDT. Every read/write cycle will change the date by 7 hours. Commiting this patch, while not covering all date cases, will fix the ones that are currently broken.
Brian, I am quite willing to include this patch with 1.1 final release but at the moment I am not able to replay the problem even with the test case you provided. Attached a new patch including your test case.
Hi Ralf,
Sorry in the delay getting back to you. Anyway when you ran the test case did you add this line to 'castor.properties':
org.exolab.castor.jdo.defaultTimeZone=UTC
If you do this the test case I provided should fail (assuming you aren't actually in the UTC timezone)
Now that you mention the timezone setting in castor.properties I know what I missed
Will come back with test updated result soon.
Hi Brian,
after trying lots of different things I finaly managed to reproduce the problem and also verified that the solution is valid. The reason for my problems was that mysql driver seams to be a bit more inteligent with regard to timezones, as this driver corrects the timezone mismatch himself. As I have no access to microsoft sql server I used maxdb instead which showed the same problem you have with the microsoft one.
After cleanup and improvement of the test case I'll commit the patch. Having said that I don't think we can integrate the test case with our suite at the moment as we have no possibility to change properties for a single test.
Hi Ralf,
Thanks for going through the contortions deed to run this test. This bug has been impacting me in a very painful ways, so I'm very relieved that you'll commit the patch.
BTW, I have run this same test on Apache Derby, Oracle 10g and XE as well as SQL Server 2000 and they have all exhibited this time zone issue. That's very interesting about the MySQL driver though; I'll have to remember that.
Final patch for review.
It need to be noted that I had to remove force of timezone for reading and writing SQL dates to omit problems. For SQL time and timestamp Castor will now allow to force a timezone. Still we need to find a solution to force timezone if date values are stored as long or string but that's another issue.
Thanks, Brian. In future, when attaching patches, can you please attach a unified patch only ?