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

Missing and duplicate results #22

Closed
wants to merge 2 commits into from
Closed

Conversation

dcendents
Copy link

Hi,

Using mybatis 3.2.1, I found myself in a strange situation where mybatis did not return all the objects I was expecting and the list also contained duplicates. This is using a simple query on a mapper interface that returns a list of objects (with nested result maps).

It looks like this is a new bug since mybatis 3.2.0, see issue 542: https://code.google.com/p/mybatis/issues/detail?id=542. In comment no. 1 there is an example where the blogs are split. When that happens the list contains blog 1 twice and blog 2 is ignored.

Thanks

…d and duplicate objects are returned instead.
@dcendents
Copy link
Author

The pull request will fix the problem. It now keeps track of what has been handled or not and make sure nothing is skipped or sent to the ResultHandler twice. I also took into consideration the memory management (as reported into issue #577).

There is still the scenario where the data is split while using a custom ResultHandler. With my fix the result handler will not be called for the second set of data. But I think this is an advanced usage of mybatis and it should be documented that the data needs to be sorted correctly when using a custom ResultHandler.

For normal usage where a mapper interface returns a list we can now expect to get everything correctly without having to worry about the ordering.

@emacarron
Copy link
Member

Hi Dan.

Thank you very much for the report and the test. You did a great work with that test but it is a bit difficult to read :).

Could you rewrite it to something that looks like this?
https://code.google.com/p/mybatis/wiki/Test

edit: I just recalled that there is a test for this issue in the package org.apache.ibatis.submitted.nestedresulthandler so better to add you case to it.

@dcendents
Copy link
Author

Sorry I did not realise there was a format to follow. I simply copied FastResultSetHandlerTest and started from there. I've deleted that test and added a new method to the existing NestedResultHandlerTest class instead. It should be much easier to understand.

Cheers,
Dan

@emacarron emacarron closed this in 9e19efc Mar 23, 2013
@emacarron
Copy link
Member

Hi Dan. Thank you for updating the test!

The problem with this is that, in order to process a Join, we need to hold all the data in memory, what is exactly the opposite of what the ResultHandler is for. So I am afraid we will never be able to achieve the two objectives: process an unodered join and call the ResultHandler row by row.

I was about to implement a "fake" behaviour, that is, given that to process an unordered join all the data has to be in memory, then there is no penalty (nor in memory or in speed) in waiting until all the data has been read and call the result handler then.

So I am afraid this is the only way to solve this. This is what I have committed, please give it a try and tell us how it works.

@ghost ghost assigned emacarron Mar 23, 2013
emacarron added a commit that referenced this pull request Mar 25, 2013
@emacarron
Copy link
Member

Hi again.

Now I recall we had a conversation on the dev list about this. The agreement was not to "fake" if but to throw an exception to warn the user. It was implemented in gcode:
https://code.google.com/p/mybatis/source/detail?spec=svn5463&r=5463

But finally this fix was removed prior to 3.2 release because I thought it was no needed.

Now we are back to that version. Hope this closes this issue definitely.

@dcendents
Copy link
Author

Hi,

Personally I'm not too concerned about the behavior for custom ResultHandler as we are not using any (at the moment). I do agree throwing an exception is the way to go to notify users of a possible configuration problem.

As long as we don't have to care about the ordering when using a simple mapper method (and implicit in-memory ResultHandler), which is now the case, I'm happy.

Thanks for the quick fix,
Dan

@gaogaoSpark
Copy link

gaogaoSpark commented Jun 12, 2016

Hi eveyone,
I also encountered this problem,I modify source code to solve it.
Steps are as follows:

1.requirement:

all collections must contains a column Id.
for example:

 <resultMap  id="HostmanageBeanMap" type="HostmanageBean" autoMapping="true" >
       <id column="ID" property="hrgId"/>
        <collection property="hrgRoleList" javaType="List"  ofType="HostResourceRoleBean" autoMapping="true" >
         <id column="ID" property="hhrrResRoleId"/>
        </collection>
        <collection property="hrgLabelList" javaType="List"  ofType="HostLableBean" autoMapping="true">
        <id column="ID" property="hhlId"/>
        </collection>
        <collection property="hrgHostComponent" javaType="List"  ofType="HostComponentBean" autoMapping="true">
        <id column="ID" property="hcId"/>
        </collection>
    </resultMap>

2.modify source code (DefaultResultSetHandler)

2.1.define a map
private Map<String,Object> idCache = new ConcurrentHashMap<String,Object>();
2.2.Add a method . According to resultMapId and value of ID to generate key

 private String createIdKey(ResultSetWrapper rsw, ResultMap resultMap,MetaObject metaObject) throws SQLException{
    StringBuffer idPrefix = new StringBuffer(resultMap.getId());
    List<ResultMapping> list = resultMap.getIdResultMappings();
    for(ResultMapping rm : list){
        String propertyName = rm.getProperty();
        String columnName = propertyName;
        final String property = metaObject.findProperty(propertyName, configuration.isMapUnderscoreToCamelCase());
        if (property != null && metaObject.hasSetter(property)) {
          final Class<?> propertyType = metaObject.getSetterType(property);
          if (typeHandlerRegistry.hasTypeHandler(propertyType)) {
            final TypeHandler<?> typeHandler = rsw.getTypeHandler(propertyType, columnName);
            final Object value = typeHandler.getResult(rsw.getResultSet(), columnName);
            idPrefix.append("-").append(value); 
          }
        }
    }
    return idPrefix.toString();
  }

2.3.Before return of getRowValue. we should check if resultObject exist.

   String idkey = createIdKey(rsw,resultMap,metaObject);
        if(null!=idCache.get(idkey)){
            return null;
        }else{
            idCache.put(idkey, resultObject);
        }

2.4.Finally,we should clear idCache .

   if (rowValue != null && mappedStatement.isResultOrdered() && shouldProcessMoreRows(resultContext, rowBounds)) {
      idCache.clear();
      storeObject(resultHandler, resultContext, rowValue, parentMapping, rsw.getResultSet());

    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants