DateTimeConverter is not considering timezone in "toDatabaseParam" method

[thedarkdestructor]

Dear all,

I have spent quite some time to figure out some strange behavior in sql2o DateTimeConverter.

I have seen that similar issues such as: https://github.com/aaberg/sql2o/issues/169 and https://github.com/aaberg/sql2o/issues/147

Even if those two issues have been fixed there is still a problem in the management of the timezone when a DateTime is provided as a paramenter. The metod toDatabaseParam is stripping off TimeZone info

public Object toDatabaseParam(DateTime val) {
  return new Timestamp(val.getMillis());
}

As reported in this stackoverflow thread to support timezones in prepared statements is required to use

setTimestamp(int parameterIndex, Timestamp x, Calendar cal)

So, also NoQuirks class should be changed to call the appropriate setTimestamp method, the one with the calendar.

[thedarkdestructor]

Can someone provide a feedback for this? Unless I overlooked something in the source code it seems to me quite a big issue...

[zapodot]

Hi! As the DateTimeConverter simply converts from Joda's LocalDateTime to java.sql.Timestamp which in itself does not hold any Timezone. When SQL2o is to set a Timestamp parameter on a PreparedStatement it invokes the "Quirks" implementation (default NoQuirks) which actually invokes the "setTimestamp" method on the PreparedStatement. At this point, SQL2O does not know which TimeZone should be set. You may, however, extend NoQuirks (or whatever Quirks you are using) to invoke the method suggested in the Stackoverflow thread you mentioned and set the TimeZone to whatever value you would like.

[thedarkdestructor]

Hi @zapodot thanks for your reply.

I understand that I can extend NoQuirks and then invoke the method setTimestamp that also consider the timezone. This is how I have currently fixed it.

However I believe - correct me if I'm wrong - that that DateTimeConverter is not behaving correctly. It's behavior is not symmetric in case of read and write from the database.

Let's consider this example:

1. The local timezone is CET

2. I setup sql2o to work with a DateTimeConverter specifying UTC as reference TimeZone: new DateTimeConverter(DateTimeZone.UTC)

3. I write a UTC date in the DB (e.g. 2018-03-20T08:43:13.668Z) in a DB field that does not store timezone info. At write time the method toDatabaseParam(DateTime val) is invoked. As we agreed this do not take into the account the timezone specified when instantiating the DateTimeConverter. So the date will be stored in the database converted in CET TimeZone. (so, in our example 2018-03-20T09:43:13.668).

4. I read the same field that I just wrote in the DB. In this case the method DateTime convert(Object val) is invoked. This one - differently to toDatabaseParam used during the write on the DB - consider the TimeZone provided when instantiating the DateTimeConverter: this is the asymmetry I was talking about! Below the code of the current implementation.

    public DateTime convert(Object val) throws ConverterException {
        if (val == null){
            return null;
        }
        try {
            // Joda has it's own pluggable converters infrastructure
            // it will throw IllegalArgumentException if can't convert
            // look @ org.joda.time.convert.ConverterManager
            return new LocalDateTime(val).toDateTime(timeZone);
        } catch (IllegalArgumentException ex) {
            throw new ConverterException("Error while converting type " + val.getClass().toString() + " to jodatime", ex);
        }
    }

5. Final result starting from a UTC date (2018-03-20T08:43:13.668Z) I write in the DB a CET date (2018-03-20T09:43:13.668) and when I read the same exact date I get a different UTC date 2018-03-20T09:43:13.668Z

I believe that this behavior is not correct. I would like to know your opinion to understand if I am missing something before implementing my custom extension of NoQuirks. Is there any reason behind this that I am not seeing?

[zapodot]

@thedarkdestructor You are probably right, however, that means we can not convert the Joda DateTime instance to Timestamp as this means we will lose the timezone information. This should probably be a lot easier when we move the Java compatibility to Java 8 as the time API is then greatly improved adding both the LocalDateTime and OffsetDateTime types. In your case using OffsetDateTime in your POJOs should keep the timezone information and should make it easy to serialize it to the database. I think for now as we are still in a pre-Java 8-world you should stick with your workaround and then hopefully things should be better in the future

[thedarkdestructor]

@zapodot thanks again for your feedback.

I can keep my workaround for now ok. Could this be a fix worth a pull request? I can write a fixed version of NoQuirks and DateTimeConverter or a decorator for NoQuirks.

I'm asking this because me and my company deeply use - and like - sql2o and would like to see progress and evolve in the future. So, instead of temporary workarounds would be great to have the fix on the trunk.

About the same topic what are the future plan? Are people still working on the project? Is any help needed? Are you looking for mantainers? As I said, we deeply use sql2o and we would like to understand if we can help in any way.

The problem I see from the outside is that if the library is not maintained from almost a year. If this continue maybe we will have to switch to another library and it would be really a pity.

[zapodot]

@thedarkdestructor if you have a good and general solution, then please create a pull request (if you do, please adhere to the coding guidelines ). Thank you for using and contributing to Sql2o! 👍

[thedarkdestructor]

I have to find the time to write that. I have a good solution but for our internal porting of the library. I have to write it for "plain sql2o". In the meantime my suggestion is to keep this issue open: the bug that I have described above is still there....