search

asyncer-io / r2dbc-mysql

Reactive Relational Database Connectivity for MySQL. The official successor to mirromutth/r2dbc-mysql(dev.miku:r2dbc-mysql).
https://r2dbc.io
Apache License 2.0
195 stars 21 forks source link

Discussion on Default Codecs for R2DBC MySQL #166

Closed jchrys closed 6 months ago

jchrys commented 8 months ago

I'm initiating a discussion on a crucial matter concerning the default codec for MySqlType.ID_TIMESTAMP and MySqlType.ID_DATETIME.

Currently, we're using ZonedDateTime.class as default Codec for above MySqlTypes, which has resulted in compatibility issues with the Micronaut Data (R2DBC) project, as highlighted in this pull request (https://github.com/micronaut-projects/micronaut-data/pull/2388).

Attempts to resolve this by setting LocalDateTime.class as the default codec introduced a backward compatibility problem with Spring Data, documented in the related issue (https://github.com/asyncer-io/r2dbc-mysql/issues/151), leading to a rollback to ZonedDateTime.class as the default (https://github.com/asyncer-io/r2dbc-mysql/pull/155).

I'm now contemplating setting LocalDateTime.class as the default codec after an upcoming minor version update(1.1.0/0.10.0).

(LocalDateTime.class was default in dev.miku's, and LocalDateTime.class is default in latest mariadb-r2dbc.)

I need your opinions on how best to navigate these challenges. What do you think? @mirromutth @JohnNiang

mirromutth commented 8 months ago

In fact, I have also hesitated for a long time about the default type at that time.

I also think ZonedDateTime makes more sense, but essentially the question is what does the ORM/QueryBuilder do after getting the result of DATETIME.

For example, if we are using JPA (3.2.0) + JDBC (8.2.0), create an entity with a ZonedDateTime property and try to findAll, we will see com.mysql.cj.jdbc.result.ResultSetImpl.getTimestamp(int, Calendar) (line 975) is called, it is not ResultSet.getObject(int) or ResultSet.getObject(int, Class<T>). Which means, JPA will always get java.sql.Timestamp first, then convert it to ZonedDateTime after that. Actually, mysql-connector-j returns LocalDateTime as default type of DATETIME in ResultSetImpl.getObject(int).

So LocalDateTime is the most common choice, but we also need more explanation and communication with the ORM/QueryBuilder framework maintainers to solve the overall problem.