Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OffsetDateTimeTypeHandler had a breaking change from 3.4.6 -> 3.5.3 #1751

Closed
fieldju opened this issue Nov 22, 2019 · 2 comments
Closed

OffsetDateTimeTypeHandler had a breaking change from 3.4.6 -> 3.5.3 #1751

fieldju opened this issue Nov 22, 2019 · 2 comments

Comments

@fieldju
Copy link

fieldju commented Nov 22, 2019

Java Version

OpenJdk 11.0.2

MyBatis version

3.5.3

Database vendor and version

MySql 5.7

Description

I am upgrading

FROM

"org.mybatis:mybatis:3.4.6",

TO

"org.mybatis:mybatis:3.5.3"

During that process I noticed that that the OffsetDateTimeTypeHandler seemed to have a breaking change, and or maybe a bug.

https://github.com/mybatis/mybatis-3/blob/mybatis-3.4.6/src/main/java/org/apache/ibatis/type/OffsetDateTimeTypeHandler.java

vs

https://github.com/mybatis/mybatis-3/blob/mybatis-3.5.3/src/main/java/org/apache/ibatis/type/OffsetDateTimeTypeHandler.java

It seems internal logic for handling the serialization and deserialization has been removed.

Expected result

INSERT INTO
        CATEGORY
          (
            ID,
            DISPLAY_NAME,
            PATH,
            CREATED_BY,
            LAST_UPDATED_BY,
            CREATED_TS,
            LAST_UPDATED_TS
          )
      VALUES
        (
          '64fa6430-8df9-40d0-9f3c-4caab5dfe403',
          'MyCategory',
          'mycategory',
          'TODO',
          'TODO',
          ** STREAM DATA **, <-- REAL TIMESTAMPS HERE
          ** STREAM DATA ** <-- REAL TIMESTAMPS HERE
        )

Actual result

        CATEGORY
          (
            ID,
            DISPLAY_NAME,
            PATH,
            CREATED_BY,
            LAST_UPDATED_BY,
            CREATED_TS,
            LAST_UPDATED_TS
          )
      VALUES
        (
          '64fa6430-8df9-40d0-9f3c-4caab5dfe403',
          'MyCategory',
          'mycategory',
          'TODO',
          'TODO',
          ** STREAM DATA **,
          ** STREAM DATA **
        )

Workaround

I copied the old class and stored a copy in my project and wired it up via

  @Bean
  ConfigurationCustomizer mybatisConfigurationCustomizer(@Value("${cms.mybatis.cache.enabled:#{false}}") boolean isCacheEnabled) {
    return configuration -> {
      configuration.setCacheEnabled(isCacheEnabled);
      // For some reason the built in OffsetDateTimeTypeHandler doesn't work like it did in the guice module
      configuration.getTypeHandlerRegistry()
        .register(OffsetDateTime.class, new OffsetDateTimeTypeHandler());
    };
  }
}
@harawata
Copy link
Member

harawata commented Nov 23, 2019

Hi @fieldju ,

It's not a bug.
From Java point of view, the expected behavior would be...

  1. When you persist a OffsetDateTime, the offset information should be stored in the database as well (even if it's different from the system's offset).
  2. When you persist a OffsetDateTime instance and then retrieve it, the returned OffsetDateTime should have the same offset as the inserted one (even when it's different from the system's offset).

The new implementation satisfies these conditions, but it requires the database to support storing offset information and MySQL does not support it (see #1368 (comment)).

The old implementation didn't satisfy both 1 and 2, but it may have seemed to satisfy 2 if you always used the default offset (i.e. ZoneOffset.systemDefault()).

The current workaround is to copy the old implementation to your project and register it in the config.
Please let us know if you need further assistance.

(You already did this. Forgot to edit the template :))

@harawata
Copy link
Member

harawata commented Jan 2, 2020

Closing. Please let me know if my explanation is unclear.

@harawata harawata closed this as completed Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants