MyBatis version
3.5.4
Database vendor and version
H2
Test case or example project
https://github.com/blindpirate/mybatis-bug-reproduction
Steps to reproduce
mvn flyway:migrate
- Run
Main
class:
The result set should be 3 rows, but MyBatis returns 2 rows.
Expected result
DEBUG [main] - Opening JDBC Connection
DEBUG [main] - Created connection 240166646.
DEBUG [main] - Setting autocommit to false on JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - ==> Preparing: select USER.id,USER.name,t.score_num from (select USER_ID,sum(SCORE) as score_num from `MATCH` group by USER_ID) t inner join USER on t.USER_ID=USER.ID
DEBUG [main] - ==> Parameters:
DEBUG [main] - <== Total: 3
Result size: 3
Actual result
DEBUG [main] - Opening JDBC Connection
DEBUG [main] - Created connection 240166646.
DEBUG [main] - Setting autocommit to false on JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - ==> Preparing: select USER.id,USER.name,t.score_num from (select USER_ID,sum(SCORE) as score_num from `MATCH` group by USER_ID) t inner join USER on t.USER_ID=USER.ID
DEBUG [main] - ==> Parameters:
DEBUG [main] - <== Total: 3
Result size: 2
Comment From: emacarron
We had a chat by mail. I am copying the conversation here so anyone can follow.
Imagine this two objects and this result
User -> List<Score>
Mapping would be:
<resultMap id="user" type="User">
<result property="id" column="id"/>
<result property="name" column="name"/>
<collection property="scores">
<result property="score" column="score_num"/>
</collection>
</resultMap>
id, name, score 1 , zhangsan, 1300 2 , lisi , 500 3 , wangu , 500 3, wangu , 600
Result should be this: User (1, zhongshan) -> List(Score(1300)) User (2, lisi) -> List(Score(500)) User (3, wangu) -> List(Score(500),Score(600))
How does mybatis do that?? By identifying the key of each object. In this case rows 4 and 5 contain the same object User (3, wangu). To identify that Object we cannot add the Score to the key or the result will be wrong:
User (1, zhongshan) -> List(Score(1300)) User (2, lisi) -> List(Score(500)) User (3, wangu) -> List(Score(500) User (3, wnagu) -> LIst(Score(600) -> wrong!!
In this case the 1st colum is the primary key so it would work also if you define the resultmap like this:
<resultMap id="user" type="User">
<id property="id" column="id"/>
<result property="name" column="name"/>
<collection property="scores">
<result property="score" column="score_num"/>
</collection>
</resultMap>
In this case only id will be part of the key. So mybatis won't use the name to identify user objects.
So I am afraid mybatis is working fine in your test case because for mybatis, Score=500 is only one object and mybatis cannot put more than one user in a "association".
Right mapper in your case would be:
</select>
<resultMap id="rankItem" type="RankItem">
<result property="score" column="score_num"/>
<collection property="user" javaType="User">
<result property="id" column="id"/>
<result property="name" column="name"/>
</collection>
</resultMap>
However, I think line 1054 makes no sense:
createRowKeyForMappedProperties(nestedResultMap, rsw, cacheKey, nestedResultMap.getConstructorResultMappings(),
prependPrefix(resultMapping.getColumnPrefix(), columnPrefix));
Because if child objects become part of the key there is no point in using only their constructor mappings for that and not the rest of the properties.
Honestly I do not remember why is that there. I will have a deeper look and tell once I figure it out!! :)
Comment From: emacarron
My 1st thought is to keep it simple.
You can select what "simple" resultmappings you want to add to the key by using the "id" or the "result" xml element but there is not such distinction for associations and collections.
It is clear that an child object in a collection can never participate in its parent key.
An association child object (1 to 1) could be part of parents key. But if it that is a possibility then there should be an option to add it or not (same than with simple properties). Something like "association" and "id-association".
Too complex :) In my opinión, child objects should never be part of the parents key to avoid undesired effects.
Line 1054 then... must be removed.
Comment From: Huangxuny1
I use given mapper in mybatis 3.5.4 h2 db
<resultMap id="rankItem" type="RankItem">
<result property="score" column="score_num"/>
<collection property="user" javaType="User">
<result property="id" column="id"/>
<result property="name" column="name"/>
</collection>
</resultMap>
the table is :
USER
id | name | pk |
---|---|---|
1 | zhangsan | Y |
2 | lisi | |
3 | wangwu |
MATCH
id | user_id | score | pk |
---|---|---|---|
1 | 1 | 1000 | Y |
2 | 1 | 300 | |
3 | 2 | 500 | |
4 | 3 | 500 |
Result size: 2
RankItem{user=User{id=1, name='zhangsan'}, score=1300}
RankItem{user=User{id=3, name='wangwu'}, score=500}
so it still have some issues
Comment From: emacarron
Hi Xunyi,
I would say it works as expected but it cannot run that query easily inside my mind :) Can you either post the log in TRACE mode so we can see the results or upload a test to your repo?
Thanks in advance!
Comment From: Huangxuny1
Hi Xunyi,
I would say it works as expected but it cannot run that query easily inside my mind :) Can you either post the log in TRACE mode so we can see the results or upload a test to your repo?
Thanks in advance!
DEBUG [main] - Logging initialized using 'class org.apache.ibatis.logging.log4j.Log4jImpl' adapter.
DEBUG [main] - Logging initialized using 'class org.apache.ibatis.logging.log4j.Log4jImpl' adapter.
DEBUG [main] - PooledDataSource forcefully closed/removed all connections.
DEBUG [main] - PooledDataSource forcefully closed/removed all connections.
DEBUG [main] - PooledDataSource forcefully closed/removed all connections.
DEBUG [main] - PooledDataSource forcefully closed/removed all connections.
DEBUG [main] - Opening JDBC Connection
DEBUG [main] - Created connection 788625466.
DEBUG [main] - Setting autocommit to false on JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - ==> Preparing: select USER.id,USER.name,t.score_num from (select USER_ID,sum(SCORE) as score_num from `MATCH` group by USER_ID) t inner join USER on t.USER_ID=USER.ID
DEBUG [main] - ==> Parameters:
TRACE [main] - <== Columns: ID, NAME, SCORE_NUM
TRACE [main] - <== Row: 1, zhangsan, 1300
TRACE [main] - <== Row: 2, lisi, 500
TRACE [main] - <== Row: 3, wangwu, 500
DEBUG [main] - <== Total: 3
Result size: 2
RankItem{user=User{id=1, name='zhangsan'}, score=1300}
RankItem{user=User{id=3, name='wangwu'}, score=500}
DEBUG [main] - Resetting autocommit to true on JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - Closing JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - Returned connection 788625466 to pool.
That's the log ,
and https://github.com/blindpirate/mybatis-bug-reproduction can reproduce
Thanks
Comment From: emacarron
Change this;
private User user;
by this:
private List<User> user;
Comment From: Huangxuny1
Change this;
private User user;
by this:
private List<User> user;
DEBUG [main] - Opening JDBC Connection
DEBUG [main] - Created connection 788625466.
DEBUG [main] - Setting autocommit to false on JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - ==> Preparing: select USER.id,USER.name,t.score_num from (select USER_ID,sum(SCORE) as score_num from `MATCH` group by USER_ID) t inner join USER on t.USER_ID=USER.ID
DEBUG [main] - ==> Parameters:
DEBUG [main] - Resetting autocommit to true on JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - Closing JDBC Connection [conn0: url=jdbc:h2:file:./target/test user=ROOT]
DEBUG [main] - Returned connection 788625466 to pool.
Exception in thread "main" org.apache.ibatis.exceptions.PersistenceException:
### Error querying database. Cause: org.apache.ibatis.reflection.ReflectionException: Could not set property 'user' of 'class RankItem' with value 'User{id=1, name='zhangsan'}' Cause: java.lang.IllegalArgumentException: argument type mismatch
### The error may exist in db/mybatis/MyMapper.xml
### The error may involve MyMapper.selectRank
### The error occurred while handling results
### SQL: select USER.id,USER.name,t.score_num from (select USER_ID,sum(SCORE) as score_num from `MATCH` group by USER_ID) t inner join USER on t.USER_ID=USER.ID
### Cause: org.apache.ibatis.reflection.ReflectionException: Could not set property 'user' of 'class RankItem' with value 'User{id=1, name='zhangsan'}' Cause: java.lang.IllegalArgumentException: argument type mismatch
at org.apache.ibatis.exceptions.ExceptionFactory.wrapException(ExceptionFactory.java:30)
at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:149)
at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:140)
at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:135)
at Main.main(Main.java:15)
Caused by: org.apache.ibatis.reflection.ReflectionException: Could not set property 'user' of 'class RankItem' with value 'User{id=1, name='zhangsan'}' Cause: java.lang.IllegalArgumentException: argument type mismatch
at org.apache.ibatis.reflection.wrapper.BeanWrapper.setBeanProperty(BeanWrapper.java:185)
at org.apache.ibatis.reflection.wrapper.BeanWrapper.set(BeanWrapper.java:59)
at org.apache.ibatis.reflection.MetaObject.setValue(MetaObject.java:140)
at org.apache.ibatis.executor.resultset.DefaultResultSetHandler.linkObjects(DefaultResultSetHandler.java:1112)
at org.apache.ibatis.executor.resultset.DefaultResultSetHandler.applyNestedResultMappings(DefaultResultSetHandler.java:954)
at org.apache.ibatis.executor.resultset.DefaultResultSetHandler.getRowValue(DefaultResultSetHandler.java:907)
at org.apache.ibatis.executor.resultset.DefaultResultSetHandler.handleRowValuesForNestedResultMap(DefaultResultSetHandler.java:870)
at org.apache.ibatis.executor.resultset.DefaultResultSetHandler.handleRowValues(DefaultResultSetHandler.java:326)
at org.apache.ibatis.executor.resultset.DefaultResultSetHandler.handleResultSet(DefaultResultSetHandler.java:301)
at org.apache.ibatis.executor.resultset.DefaultResultSetHandler.handleResultSets(DefaultResultSetHandler.java:194)
at org.apache.ibatis.executor.statement.PreparedStatementHandler.query(PreparedStatementHandler.java:65)
at org.apache.ibatis.executor.statement.RoutingStatementHandler.query(RoutingStatementHandler.java:79)
at org.apache.ibatis.executor.SimpleExecutor.doQuery(SimpleExecutor.java:63)
at org.apache.ibatis.executor.BaseExecutor.queryFromDatabase(BaseExecutor.java:324)
at org.apache.ibatis.executor.BaseExecutor.query(BaseExecutor.java:156)
at org.apache.ibatis.executor.CachingExecutor.query(CachingExecutor.java:109)
at org.apache.ibatis.executor.CachingExecutor.query(CachingExecutor.java:83)
at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:147)
... 3 more
Caused by: java.lang.IllegalArgumentException: argument type mismatch
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.apache.ibatis.reflection.invoker.MethodInvoker.invoke(MethodInvoker.java:44)
at org.apache.ibatis.reflection.wrapper.BeanWrapper.setBeanProperty(BeanWrapper.java:180)
... 20 more
Process finished with exit code 1
Comment From: emacarron
You probably need a setter:
public void setUser(List<User> user) {
this.user = user;
}
Comment From: emacarron
@harawata help me find the reason that line was there: "https://github.com/mybatis/old-google-code-issues/issues/392"
I am reopening the issue until this is clarified.
Comment From: emacarron
These two tests were removed (thanks Iwao) so I added them back again in https://github.com/mybatis/mybatis-3/commit/796cf2f1fe73fcdbd42c9f51a4a3fbaf9c2d2477
Original tests where these (note results are 4, not 2).
@Test
public void shouldFindAllPostLites() throws Exception {
List<PostLite> posts = manager.selectList("org.apache.ibatis.domain.blog.mappers.PostMapper.selectPostLite");
assertEquals(4, posts.size());
}
@Test
public void shouldFindAllMutablePostLites() throws Exception {
List<PostLite> posts = manager.selectList("org.apache.ibatis.domain.blog.mappers.PostMapper.selectMutablePostLite");
assertEquals(4, posts.size());
}
They fail with this fix "current version" and worked fine with previous version.
However seems that shouldFindAllMutablePostLites contained a typo in the mapper. It was:
<resultMap id="mutablePostLiteMap" type="org.apache.ibatis.domain.blog.PostLite">
<result property="blogId" column="blog_id"/>
<association property="id" column="id" resultMap="postLiteIdMap"/>
</resultMap>
And seems that original intention was to be like follows, otherwise mutablePostLiteIdMap would be not used:
<resultMap id="mutablePostLiteMap" type="org.apache.ibatis.domain.blog.PostLite">
<result property="blogId" column="blog_id"/>
<association property="id" resultMap="mutablePostLiteIdMap"/>
</resultMap>
With this change and the previous version of DefaultResultSetHandler, the test fails with: org.opentest4j.AssertionFailedError: expected: <4> but was: <2>
Previous version of the test worked not because PostLite has constructor mappings but because the associated "child" object PostLiteId has constructor mappings.
In order to make the test work with original intention code in DefaultResultSetHandler should probably be this:
if (resultMapping.getNestedResultMapId() != null && resultMapping.getFlags().contains(ResultFlag.CONSTRUCTOR)) {
// Issue #392
final ResultMap nestedResultMap = configuration.getResultMap(resultMapping.getNestedResultMapId());
createRowKeyForMappedProperties(nestedResultMap, rsw, cacheKey, getResultMappingsForRowKey(resultMap),
prependPrefix(resultMapping.getColumnPrefix(), columnPrefix));
In my opinion current version is right. The rule should be that child objects (associations/collections) do not participate in the cache key and should not matter if they are injected as part of the constructor. Any other combination would make the rule impossible to remember.
Comment From: harawata
The behavior reported in this issue does not seem like a bug to me.
@Huangxuny1 ,
When using <collection />
or <association />
, it is very important to understand how MyBatis identifies the parent object.
Here is your result map:
<resultMap id="rankItem" type="RankItem">
<result property="score" column="score_num"/>
<association property="user" javaType="User">
<result property="id" column="id"/>
<result property="name" column="name"/>
</association>
</resultMap>
As there is no <id />
in the root level, MyBatis uses score_num
as the ID of the root object (= RankItem
) and associates User
for each RankItem
[1].
The example query result is:
ID | NAME | SCORE_NUM |
---|---|---|
1 | zhangsan | 1300 |
2 | lisi | 500 |
3 | wangwu | 500 |
As I explained, score_num
is the ID of RankItem
, so there will be two RankItem
s (score=1300 and score=500) instead of 3.
This is how it happens.
Now, assuming you actually want one RankItem
for each User
, you need to tell MyBatis to use the column that identifies USER
(i.e USER.id
) to identify RankItem
.
To do this, you just need to add <id />
to your result map.
<resultMap id="rankItem" type="RankItem">
<id column="id" /><!-- added -->
<result property="score" column="score_num"/>
<association property="user" javaType="User">
<result property="id" column="id"/>
<result property="name" column="name"/>
</association>
</resultMap>
It's pretty much the same with <collection />
.
<resultMap id="rankItem" type="RankItem">
<id column="id" /><!-- added -->
<result property="score" column="score_num"/>
<collection property="user" javaType="list" ofType="User">
<result property="id" column="id"/>
<result property="name" column="name"/>
</association>
</resultMap>
Notes:
- I didn't specify
property
in the<id />
element as there is no corresponding property inRankItem
. - You should specify
<id />
in<collection />
or<association />
as well (i.e.<id property="id" column="id"/>
). It helps MyBatis work more efficiently and is mandatory in advanced (=complex) mappings. This is mentioned in the doc.
Please try it and let us know if there is any further questions.
[1] When there is no <id />
element, MyBatis uses the combination of all <result />
elements as the ID.
@emacarron , @h3adache
I am not so sure, but I think that the line 1054 is for a result map that has <constructor />
in it.
I'll take a deeper look once I find some more time.
Comment From: Huangxuny1
I have read these issue
is this bug? #512
Some records are missing in result #580
mybatis lose duplicated rows when using association #522
mches said : For result maps with associations and collections MyBatis de-duplicates the result set according to the
columns if present, otherwise every column.
and I know in my demo it has workaround .
but in my opinion , join tableA and tableB , sometimes there is no "primary key" or "unique key" ,
in that condition , the best way to calc CacheKey
is let every field to update cache key ..
otherwise , it will has record missing ...
No matter how mybatis optimizes, it should not lose unique data from jdbc query
at first , I speculate it is createRowKeyForMappedProperties(nestedResultMap, rsw, cacheKey, nestedResultMap.getConstructorResultMappings(),
prependPrefix(resultMapping.getColumnPrefix(), columnPrefix));
nestedResultMap.getConstructorResultMappings()
issue ,
so I change nestedResultMap.getConstructorResultMappings()
to nestedResultMap.getPropertyResultMappings()
but i am not sure so I send a email to ask Mr.emacarron ,He is very patient and friendly .
It turns out I was wrong , the unit test failed
he gave code that solves my problem
private void createRowKeyForMappedProperties(ResultMap resultMap, ResultSetWrapper rsw, CacheKey cacheKey, String columnPrefix) throws SQLException {
List<ResultMapping> resultMappings = getResultMappingsForRowKey(resultMap);
for (ResultMapping resultMapping : resultMappings) {
if (resultMapping.getNestedResultMapId() != null && resultMapping.getResultSet() == null) {
// Issue #392
final ResultMap nestedResultMap = configuration.getResultMap(resultMapping.getNestedResultMapId());
createRowKeyForMappedProperties(nestedResultMap, rsw, cacheKey, prependPrefix(resultMapping.getColumnPrefix(), columnPrefix));
} else if (resultMapping.getNestedQueryId() == null) {
final String column = prependPrefix(resultMapping.getColumn(), columnPrefix);
final TypeHandler<?> th = resultMapping.getTypeHandler();
List<String> mappedColumnNames = rsw.getMappedColumnNames(resultMap, columnPrefix);
// Issue #114
if (column != null && mappedColumnNames.contains(column.toUpperCase(Locale.ENGLISH))) {
final Object value = th.getResult(rsw.getResultSet(), column);
if (value != null || configuration.isReturnInstanceForEmptyRow()) {
cacheKey.update(column);
cacheKey.update(value);
}
}
}
}
}
but It's fail in unit test
Permissions#checkNestedResultMapLoop works with my fix because it mapper is written like this:
<resultMap id="resourceResults" type="Resource">
<id property="name" column="resourceName" />
<collection property="principals" resultMap="principalResults" />
</resultMap>
<resultMap id="principalResults" type="Principal">
<id property="principalName" column="principalName" />
<collection property="permissions" resultMap="permisionResults" />
</resultMap>
<resultMap id="permisionResults" type="Permission">
<result property="permission" column="permission" />
<association property="resource" resultMap="resourceResults" />
</resultMap>
But fails if written like this:
<resultMap id="resourceResults" type="Resource">
<result property="name" column="resourceName" />
<collection property="principals" resultMap="principalResults" />
</resultMap>
<resultMap id="principalResults" type="Principal">
<result property="principalName" column="principalName" />
<collection property="permissions" resultMap="permisionResults" />
</resultMap>
<resultMap id="permisionResults" type="Permission">
<result property="permission" column="permission" />
<association property="resource" resultMap="resourceResults" />
</resultMap>
The reason is that if you used the "id" element MyBatis will only use that mapping to build the cache key but when using "result" it uses all mappings. If you follow the nested mappings you will find a loop principal->permission->resource->principal->permissions->resource .... etc...
and I am suggest add this mapper into unit test
and back to my opinion ... I think CacheKey should calculate from inside to outside ,(eg. User->RankItem) or It could use CombineKey
to make sure if is unique data
Comment From: harawata
@Huangxuny1 ,
Implementations aside, it would be logically impossible to map complex object graph from joined results without knowing the parent's ID.
Taking the example result of USER-MATCH tables...
ID | NAME | SCORE_NUM |
---|---|---|
1 | zhangsan | 1300 |
2 | lisi | 500 |
3 | wangwu | 500 |
You seem to assume that ID
is the only possible key of the RankItem
, but that is only because you know the result you want.
What if you wanted a list of users per score? i.e.
- Users who scored 500 = [2:lisi] and [3:wongwu]
- Users who scored 1300 = [1:zhangson]
In this case, the key is SCORE_NUM
and the result is totally valid.
My point is that MyBatis cannot read your thoughts, so you need to let MyBatis know which columns to use as the parent key. Makes sense?
Comment From: Huangxuny1
OK, thanks
On Thu, Mar 12, 2020 at 11:40 PM Iwao AVE! notifications@github.com wrote:
@Huangxuny1 https://github.com/Huangxuny1 ,
Implementations aside, it would be logically impossible to map complex object graph from joined results without knowing the parent's ID.
Taking the example result of USER-MATCH tables... ID NAME SCORE_NUM 1 zhangsan 1300 2 lisi 500 3 wangwu 500
You seem to assume that ID is the only possible key of the RankItem, but that is only because you know the result you want. What if you wanted a list of users per score? i.e.
- Users who scored 500 = [2:lisi] and [3:wongwu]
- Users who scored 1300 = [1:zhangson]
This is a totally valid result.
My point is that MyBatis cannot read your thoughts, so you need to let MyBatis know which columns to use as the parent key. Makes sense?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mybatis/mybatis-3/issues/1848#issuecomment-598257925, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFHWTJS5ICWNYJFKIP3U56DRHD67XANCNFSM4LDEU4JA .
Comment From: emacarron
Hi @harawata, reviewing the issue, I should probably have added more context to my 1st message.
@Huangxuny1 sent me a mail pointing to an old hidden error in line 1054. That got my attention because that sounded cool ;) but also because that is indeed a weird block of code.
I already told him that his issue was not so but also to keep this issue open to start the discussion about line 1054.
As I told up there, I think that code was wrong. It is worth also knowing that may break existing code but should be very rare.
If you don't mind I would prefer to wait untill you have some time to review the change before closing the issue.
Comment From: harawata
@emacarron , Of course. Thank you for the info and I'm sorry about the delay! Please give me a few more days.
Comment From: harawata
@emacarron
I read the code and agree with you on your following statement.
The rule should be that child objects (associations/collections) do not participate in the cache key and should not matter if they are injected as part of the constructor.
It might break existing code, but it should be possible to fix the breakage by specifying property-less-id elements as I explained above.
Comment From: emacarron
Thank you both!