java.jdbc

Wrong timezone for java.sql.Date, java.sql.Time and java.sql.Timestamp objects returned by with-query-results

Details

  • Type: Defect Defect
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:

Description

Our database stores date and time related information in UTC. But, the results from with-query-results creates objects in the local timezone. This is causing a lot of unexpected behavior when constructing Joda DateTime objects in clj-time. Currently, the with-query-results function is using the single argument getter functions in the ResultSet

getDate(int columnIndex)
getTime(int columnIndex)
getTimestamp(int columnIndex)

We can solve this problem if we can optionally pass the timezone information and call the two argument getter functions.

getDate(int columnIndex, Calendar cal)
getTime(int columnIndex, Calendar cal)
getTimestamp(int columnIndex, Calendar cal)

Activity

Hide
Sean Corfield added a comment -

The java.jdbc docs are open for community contributions so you can propose changes to the Using SQL > Protocol Extensions section:

http://clojure-doc.org/articles/ecosystem/java_jdbc/using_sql.html#protocol-extensions-for-transforming-values

Show
Sean Corfield added a comment - The java.jdbc docs are open for community contributions so you can propose changes to the Using SQL > Protocol Extensions section: http://clojure-doc.org/articles/ecosystem/java_jdbc/using_sql.html#protocol-extensions-for-transforming-values
Hide
George Fraser added a comment -

I dug into this a bit more and I see that it is basically a problem with JDBC: java.sql.Date extends java.util.Date, using the system default time zone to fill in the missing information. This is especially problematic from the point of view of clojure because they get printed as `#inst` so it appears we are working with regular Date objects. The fix is just to extend the `IResultSetReadColumn` protocol that you have so helpfully provided, for example to use JSR-310 dates:

```
(extend-protocol jdbc/IResultSetReadColumn
java.sql.Date
(result-set-read-column [val _ _]
(.toLocalDate val))

java.sql.Timestamp
(result-set-read-column [val _ _]
(.toInstant val)))
```

Unfortunately, since there are currently several competing Java Date solutions (Joda, JSR-310, Date/GregorianCalendar) there isn't a single default that will work for everyone. But I would suggest that you put something in the readme that anyone who intends to work with dates should extend the IResultSetReadColumn protocol to convert to their preferred representation.

Show
George Fraser added a comment - I dug into this a bit more and I see that it is basically a problem with JDBC: java.sql.Date extends java.util.Date, using the system default time zone to fill in the missing information. This is especially problematic from the point of view of clojure because they get printed as `#inst` so it appears we are working with regular Date objects. The fix is just to extend the `IResultSetReadColumn` protocol that you have so helpfully provided, for example to use JSR-310 dates: ``` (extend-protocol jdbc/IResultSetReadColumn java.sql.Date (result-set-read-column [val _ _] (.toLocalDate val)) java.sql.Timestamp (result-set-read-column [val _ _] (.toInstant val))) ``` Unfortunately, since there are currently several competing Java Date solutions (Joda, JSR-310, Date/GregorianCalendar) there isn't a single default that will work for everyone. But I would suggest that you put something in the readme that anyone who intends to work with dates should extend the IResultSetReadColumn protocol to convert to their preferred representation.
Hide
Sean Corfield added a comment -

clojure.java.jdbc does absolutely nothing with date or timestamp columns: it hands back exactly what it gets from the underlying Java JDBC library, so this is absolutely NOT a bug in clojure.java.jdbc (there are no references to date or time in the code base). If you think it's a bug, petition the underlying JDBC libraries and see what they say...

Show
Sean Corfield added a comment - clojure.java.jdbc does absolutely nothing with date or timestamp columns: it hands back exactly what it gets from the underlying Java JDBC library, so this is absolutely NOT a bug in clojure.java.jdbc (there are no references to date or time in the code base). If you think it's a bug, petition the underlying JDBC libraries and see what they say...
Hide
George Fraser added a comment -

I have just encountered this issue and I cannot believe you don't consider this a bug. Coercing all DATE fields to the local timezone is literally wrong. You could justify the DB timezone, or a Calendar object, or JSR-310, but this is just a big ball of surprising behavior.

Show
George Fraser added a comment - I have just encountered this issue and I cannot believe you don't consider this a bug. Coercing all DATE fields to the local timezone is literally wrong. You could justify the DB timezone, or a Calendar object, or JSR-310, but this is just a big ball of surprising behavior.
Hide
Sean Corfield added a comment -

Since the proposed patch requires the user to pass in a list of columns to treat specially - and reorders columns in results and adds some performance overhead for all users, not just those wanting to adjust column values - I believe users who choose to run JDBC clients in timezones different to the database, against the widely-listed best practice recommendations, should bear the burden of adjusting the columns themselves in their own client code. Since they already know which columns to adjust and they know the difference between UTC and local time, they should be able to make the adjustments easily enough as part of post-processing the resultset-seq.

Show
Sean Corfield added a comment - Since the proposed patch requires the user to pass in a list of columns to treat specially - and reorders columns in results and adds some performance overhead for all users, not just those wanting to adjust column values - I believe users who choose to run JDBC clients in timezones different to the database, against the widely-listed best practice recommendations, should bear the burden of adjusting the columns themselves in their own client code. Since they already know which columns to adjust and they know the difference between UTC and local time, they should be able to make the adjustments easily enough as part of post-processing the resultset-seq.
Hide
Jestine Paul added a comment -

As mentioned earlier, the database server timezone is in UTC and the JDBC library runs on a client machine set to local time. If the database has a date value of 2012-7-23, it changes to 2012-7-22 on the client side (if the client is running at timezone greater than UTC) when coerced using to-date-time in clj-time. This is extremely dangerous and is not specific to any database. I noticed it first on Sybase Enterprise Server and I have now also replicated it in the test case with Postgresql.

I have attached a patch which fixes this problem by passing in an optional parameter. The test case is also modified to use clj-time, as it expresses the problem more clearly. Please let me know if you need any more clarification.

p.s. I have already mailed the CA and should reach Durham in a few days.

Show
Jestine Paul added a comment - As mentioned earlier, the database server timezone is in UTC and the JDBC library runs on a client machine set to local time. If the database has a date value of 2012-7-23, it changes to 2012-7-22 on the client side (if the client is running at timezone greater than UTC) when coerced using to-date-time in clj-time. This is extremely dangerous and is not specific to any database. I noticed it first on Sybase Enterprise Server and I have now also replicated it in the test case with Postgresql. I have attached a patch which fixes this problem by passing in an optional parameter. The test case is also modified to use clj-time, as it expresses the problem more clearly. Please let me know if you need any more clarification. p.s. I have already mailed the CA and should reach Durham in a few days.
Hide
Sean Corfield added a comment -

Looking at recommended practices out there, such as listed here: http://stackoverflow.com/questions/2532729/daylight-saving-time-and-timezone-best-practices one important thing is: "On Servers, set hardware clocks and OS clocks to UTC. Use NTP services on all servers." The same advice is repeated here: http://www.dbspecialists.com/blog/database-theory/intelligent-date-handling/

Looking further into PostgreSQL (a database I generally don't use), I see this: http://stackoverflow.com/questions/6151084/which-timestamp-type-to-choose-in-a-postgresql-database which seems to contain a lot of PostgreSQL-specific stuff. However, that article makes it clear that you need to set the database timezone to UTC in order to SELECT timestamp columns correctly.

In my opinion, the behavior you're seeing is not a bug (in java.jdbc) but an artifact of your environment being set up incorrectly. I'll leave this ticket open for a little while for more discussion but without a concrete patch that is shown to not affect other use cases, I will close this ticket by the end of August.

Show
Sean Corfield added a comment - Looking at recommended practices out there, such as listed here: http://stackoverflow.com/questions/2532729/daylight-saving-time-and-timezone-best-practices one important thing is: "On Servers, set hardware clocks and OS clocks to UTC. Use NTP services on all servers." The same advice is repeated here: http://www.dbspecialists.com/blog/database-theory/intelligent-date-handling/ Looking further into PostgreSQL (a database I generally don't use), I see this: http://stackoverflow.com/questions/6151084/which-timestamp-type-to-choose-in-a-postgresql-database which seems to contain a lot of PostgreSQL-specific stuff. However, that article makes it clear that you need to set the database timezone to UTC in order to SELECT timestamp columns correctly. In my opinion, the behavior you're seeing is not a bug (in java.jdbc) but an artifact of your environment being set up incorrectly. I'll leave this ticket open for a little while for more discussion but without a concrete patch that is shown to not affect other use cases, I will close this ticket by the end of August.
Hide
Jestine Paul added a comment -

I have added a new test and it is failing with my Postgresql.

https://github.com/jestinepaul/java.jdbc/commit/195397b1b2a0245d2439ab9963fe2138450a27f3

Show
Jestine Paul added a comment - I have added a new test and it is failing with my Postgresql. https://github.com/jestinepaul/java.jdbc/commit/195397b1b2a0245d2439ab9963fe2138450a27f3
Hide
Sean Corfield added a comment -

Can you provide a self-contained test case? Lots of people are using java.jdbc in production without running into this problem, and at World Singles we've had this in production for a long time in a high traffic environment without seeing any problems with timezones. Dates go in and out of the database unchanged, which is exactly as expected - and we have databases running in three different timezones.

Show
Sean Corfield added a comment - Can you provide a self-contained test case? Lots of people are using java.jdbc in production without running into this problem, and at World Singles we've had this in production for a long time in a high traffic environment without seeing any problems with timezones. Dates go in and out of the database unchanged, which is exactly as expected - and we have databases running in three different timezones.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: