Closes #22. Lost rows when handling an unordered join with a RowHandler

Author: emacarron

src/main/java/org/apache/ibatis/executor/resultset/NestedResultSetHandler.java

@@ -17,6 +17,7 @@
 
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -44,6 +45,7 @@ public class NestedResultSetHandler extends FastResultSetHandler {
 
   private final Map<CacheKey, Object> objectCache = new HashMap<CacheKey, Object>();
   private final Map<CacheKey, Object> ancestorCache = new HashMap<CacheKey, Object>();
+  private final List<Object> unsortedRowHandlerElements = new ArrayList<Object>();
 
   public NestedResultSetHandler(Executor executor, MappedStatement mappedStatement, ParameterHandler parameterHandler, ResultHandler resultHandler, BoundSql boundSql, RowBounds rowBounds) {
     super(executor, mappedStatement, parameterHandler, resultHandler, boundSql, rowBounds);
@@ -67,6 +69,7 @@ private void ensureNoRowBounds(RowBounds rowBounds) {
   protected void cleanUpAfterHandlingResultSet() {
     super.cleanUpAfterHandlingResultSet();
     objectCache.clear();
+    unsortedRowHandlerElements.clear();
   }
 
   //
@@ -82,13 +85,26 @@ protected void handleRowValues(ResultSet rs, ResultMap resultMap, ResultHandler
       final ResultMap discriminatedResultMap = resolveDiscriminatedResultMap(rs, resultMap, null);
       final CacheKey rowKey = createRowKey(discriminatedResultMap, rs, null, resultColumnCache);
       Object partialObject = objectCache.get(rowKey);
-      if (partialObject == null && rowValue != null) { // issue #542 delay calling ResultHandler until object ends
-        if (mappedStatement.isResultOrdered()) objectCache.clear(); // issue #577 clear memory if ordered
-        callResultHandler(resultHandler, resultContext, rowValue);
-      } 
-      rowValue = getRowValue(rs, discriminatedResultMap, rowKey, rowKey, null, resultColumnCache, partialObject);
+      if (mappedStatement.isResultOrdered()) { // issue #577       
+        if (partialObject == null && rowValue != null) {
+          objectCache.clear(); 
+          callResultHandler(resultHandler, resultContext, rowValue);
+        } 
+        rowValue = getRowValue(rs, discriminatedResultMap, rowKey, rowKey, null, resultColumnCache, partialObject);
+      } else {
+        rowValue = getRowValue(rs, discriminatedResultMap, rowKey, rowKey, null, resultColumnCache, partialObject);
+        if (partialObject == null) {
+          unsortedRowHandlerElements.add(rowValue); // issue #542
+        }
+      }
+    }
+    if (mappedStatement.isResultOrdered()) {
+      if (rowValue != null) callResultHandler(resultHandler, resultContext, rowValue);
+    } else {
+      for (Object o : unsortedRowHandlerElements) {
+        callResultHandler(resultHandler, resultContext, o);
+      }
     }
-    if (rowValue != null) callResultHandler(resultHandler, resultContext, rowValue);
   }
 
   //

src/test/java/org/apache/ibatis/submitted/nestedresulthandler/Mapper.java

@@ -19,4 +19,5 @@
 
 public interface Mapper {
   List<Person> getPersons();
+  List<Person> getPersonsWithItemsOrdered();
 }

src/test/java/org/apache/ibatis/submitted/nestedresulthandler/Mapper.xml

@@ -29,8 +29,15 @@
 	</resultMap>
 
 	<select id="getPersons" resultMap="personResult">
-		select p.id as person_id, p.name as person_name, i.id as item_id, i.name as item_name 
-		from persons p, items i 
+		select p.id as person_id, p.name as person_name, i.id as item_id, i.name as item_name
+		from persons p, items i
 		where p.id = i.owner
 	</select>
-</mapper>
+
+	<select id="getPersonsWithItemsOrdered" resultMap="personResult">
+		select p.id as person_id, p.name as person_name, i.id as item_id, i.name as item_name
+		from persons p, items i
+		where p.id = i.owner
+		order by i.name
+	</select>
+</mapper>

src/test/java/org/apache/ibatis/submitted/nestedresulthandler/NestedResultHandlerTest.java

@@ -58,7 +58,7 @@ public void testGetPerson() {
       Mapper mapper = sqlSession.getMapper(Mapper.class);
 
       List<Person> persons = mapper.getPersons();
-      
+
       Person person = persons.get(0);
       Assert.assertEquals("grandma", person.getName());
       Assert.assertTrue(person.owns("book"));
@@ -79,8 +79,9 @@ public void testGetPerson() {
       sqlSession.close();
     }
   }
-  
-  @Test // issue #542
+
+  @Test
+  // issue #542
   public void testGetPersonWithHandler() {
     SqlSession sqlSession = sqlSessionFactory.openSession();
     try {
@@ -91,10 +92,44 @@ public void handleResult(ResultContext context) {
             Assert.assertEquals(2, person.getItems().size());
           }
         }
-      });      
+      });
     } finally {
       sqlSession.close();
     }
   }
-  
+
+  /**
+   * Fix bug caused by issue #542, see new issue #22 on github If we order by a
+   * nested result map attribute we can miss some records and end up with
+   * duplicates instead.
+   */
+  @Test
+  public void testGetPersonOrderedByItem() {
+    SqlSession sqlSession = sqlSessionFactory.openSession();
+    try {
+      Mapper mapper = sqlSession.getMapper(Mapper.class);
+
+      List<Person> persons = mapper.getPersonsWithItemsOrdered();
+
+      Person person = persons.get(0);
+      Assert.assertEquals("grandma", person.getName());
+      Assert.assertTrue(person.owns("book"));
+      Assert.assertTrue(person.owns("tv"));
+      Assert.assertEquals(2, person.getItems().size());
+
+      person = persons.get(1);
+      Assert.assertEquals("brother", person.getName());
+      Assert.assertTrue(person.owns("car"));
+      Assert.assertEquals(1, person.getItems().size());
+
+      person = persons.get(2);
+      Assert.assertEquals("sister", person.getName());
+      Assert.assertTrue(person.owns("phone"));
+      Assert.assertTrue(person.owns("shoes"));
+      Assert.assertEquals(2, person.getItems().size());
+    } finally {
+      sqlSession.close();
+    }
+  }
+
 }