jdbi / jdbi

The Jdbi library provides convenient, idiomatic access to relational databases in Java and other JVM technologies such as Kotlin, Clojure or Scala.
http://jdbi.org/
Apache License 2.0
1.99k stars 343 forks source link

DST issue with LocalDate and Declarative API #1806

Open ngeor opened 3 years ago

ngeor commented 3 years ago

Hello,

I experienced the following daylight savings time (DST) issue when using jdbi3 with LocalDate and declarative api.

We have a MySQL table, which has a couple of DATE columns. On the Java side, we have the counterpart pojo with LocalDate fields. As this stores historical data, users can go back in time. We noticed today that when going back before October 25th 2020, the dates get shifted by one day (e.g. the database holds the value 2020-10-19, which is a Monday, but Java reports the value 2020-10-18, which is a Sunday).

Diving into jdbi code, I think the issue starts here https://github.com/jdbi/jdbi/blob/master/core/src/main/java/org/jdbi/v3/core/mapper/JavaTimeMapperFactory.java#L73 . LocalDates seem to be "deserialized" by reading a timestamp, then converting to LocalDateTime, then converting to LocalDate. Somewhere there, someone is shifting "2020-10-19" to "2020-10-18 23:00:00" (I suppose), which then loses the time part. I think it's incorrect to give it a time part to begin with, as the db simply stores "2020-10-19".

I was able to workaround this by installing this custom mapper:

        jdbi.registerColumnMapper(new ColumnMapper<LocalDate>() {
            @Override
            public LocalDate map(ResultSet r, int columnNumber, StatementContext ctx) throws SQLException {
                return LocalDate.parse(r.getString(columnNumber));
            }
        });

so I'm not blocked by the issue, but I was wondering if you think this is a bug as well and how should it be fixed.

stevenschlansker commented 3 years ago

Hi @ngeor, the MySQL driver ability to extract a LocalDate directly is relatively new and we haven't added built in support yet. We plan to ship mappers out of the box to do direct mapping, like you have. This is tracked in #1728.

alexgula commented 3 years ago

Hi, @stevenschlansker and @ngeor, I stumbled over the same problem recently and used similar solution

jdbi.registerColumnMapper(LocalDate.class, (rs, col, ctx) -> {
    final Date date = rs.getDate(col);
    return date == null ? null : date.toLocalDate();
});

Here I use java.sql.Date which is the standard JDBC feature and designed to support SQL DATE type. Do you see any flaw in this solution? If it's ok, it can be used in https://github.com/jdbi/jdbi/blob/master/core/src/main/java/org/jdbi/v3/core/mapper/JavaTimeMapperFactory.java#L73 as well, what do you think?