mybatis / mybatis-3

MyBatis SQL mapper framework for Java
http://mybatis.github.io/mybatis-3/
Apache License 2.0
19.76k stars 12.85k forks source link

Mapping into Map<String, Object> contains driver specific value classes #2216

Open filiphr opened 3 years ago

filiphr commented 3 years ago

MyBatis version

3.5.6

Database vendor and version

Oracle

Test case or example project

create table users (
                       id int,
                       name varchar(20),
                       time_ timestamp
);

insert into users (id, name, time_) values(1, 'User1', {ts '2020-04-06 12:34:50'});

A mapper that returns everything from that table into a Map<String, Object>

select *
from users

Steps to reproduce

Expected result

The mapper returns a map with a value type of java.sql.Timestamp for the time_ column.

Actual result

The mapper returns a map with a value type of oracle.jdbc.Timestamp for the time_ column.


I managed to track the reason for the oracle.jdbc.Timestamp type to here.

When fetching the TypeHandler for the property it is fetched with propertyType equal to Object.class. This then leads to calling the TypeHandlerRegistry for a mapping from Object into JdbcType.TIMESTAMP, there is no such handler and then it tries to go through the following:

https://github.com/mybatis/mybatis-3/blame/b4275bdcc851cb13f40b520a03df9650069caffe/src/main/java/org/apache/ibatis/executor/resultset/ResultSetWrapper.java#L118-L126

The javaType there is oracle.jdbc.Timestamp and the jdbcType is JdbcType.TIMESTAMP. Is there a reason why that logic is only with else if and not fallbacking to looking for a known handler for the javaType and jdbcType specifically?

If that logic is changed to:

final Class<?> javaType = resolveClass(classNames.get(index));
if (javaType != null && jdbcType != null) {
      handler = typeHandlerRegistry.getTypeHandler(javaType, jdbcType);
}

if (javaType != null && (handler == null || handler instanceof UnknownTypeHandler) {
      handler = typeHandlerRegistry.getTypeHandler(javaType);
}

if (jdbcType != null && (handler == null || handler instanceof UnknownTypeHandler) {
      handler = typeHandlerRegistry.getTypeHandler(jdbcType);
}

Then the jdbc specific type handler will be returned and the value will be of the correct type.

harawata commented 3 years ago

Hello @filiphr ,

It wouldn't do much good because there can be multiple possible Java types for TIMESTAMP. You might expect java.sql.Timestamp, but others might want java.time.LocalDateTime.

In your case, you should add the following type handler mapping to the config.

<typeHandlers>
  <typeHandler
    handler="org.apache.ibatis.type.SqlTimestampTypeHandler"
    javaType="Object" jdbcType="TIMESTAMP" />
</typeHandlers>

If you prefer Java config...

configuration.getTypeHandlerRegistry()
  .register(Object.class, JdbcType.TIMESTAMP, SqlTimestampTypeHandler.class);
filiphr commented 3 years ago

I 100% agree with you that someone might expect java.time.LocalDateTime. However, I would most certainly not expect the oracle.jdbc.TIMESTAMP.

The thing is that the current scenario works fine on most DBs (tested on Postgres, SQL Server, DB2, MySQL and H2), except on Oracle. The reason is because Oracle already does something special here.

Seems like the Spring team does something special for some of the oracle classes. See this. Is there an option for MyBatis to register the Oracle specific mappings if those classes are on the classpath, without any custom configuration in user code?

harawata commented 3 years ago

We avoid writing DB/driver specific logic like that, actually. oracle.jdbc.TIMESTAMP does not seem to be useful as a result of getObject() indeed, but it is a kind of issue that should be addressed in the driver rather than in each library/framework, in my view. Oracle driver, unfortunately, is not open-source and I'm not sure how to report issues.

Mapping between Java and JDBC types is highly dependent on the driver implementation and it could also change over time. To give you an example, mysql-connector-java has just changed a behavior in version 8.0.23 i.e. for DATETIME column, getObject() now returns java.time.LocalDateTime while older versions return java.sql.Timestamp (or java.lang.String according to the changelog). So, it may be better to specify type handlers explicitly especially if your solution is used with multiple DBs.