From 6ba272a7324e428e1541394076cdb57fdf66b83a Mon Sep 17 00:00:00 2001
From: hengsin <hengsin@gmail.com>
Date: Wed, 18 May 2022 22:20:10 +0800
Subject: [PATCH] IDEMPIERE-5014 Improve efficiency of Query for large result
 sets - using Stream (#1334)

- remove iterable() and restore the POIterator base iterate()
implementation.
---
 .../src/org/compiere/model/Query.java         | 66 +++++++++++--------
 .../org/idempiere/test/base/QueryTest.java    | 25 -------
 2 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/org.adempiere.base/src/org/compiere/model/Query.java b/org.adempiere.base/src/org/compiere/model/Query.java
index 9ad02eb43e..3520bc9638 100644
--- a/org.adempiere.base/src/org/compiere/model/Query.java
+++ b/org.adempiere.base/src/org/compiere/model/Query.java
@@ -649,24 +649,6 @@ public class Query
 		return false;
 	}
 	
-	/**
-	 * Return an Iterable implementation that can be used in a <tt>for</tt> expression. Example:
-	 * <pre>{@code
-	 * Iterable<MTable> query = new Query(...).iterable();
-	 *
-	 * for (MTable table : query) {
-	 *   // Do stuff with the element
-	 * }
-	 * </pre>
-	 *
-	 * @return Iterable
-	 * @throws DBException 
-	 */
-	public <T extends PO> Iterable<T> iterable() throws DBException
-	{
-		return () -> iterate();
-	}
-	
 	/**
 	 * Return an Stream implementation to fetch one PO at a time. This method will only create POs on-demand and
 	 * they will become eligible for garbage collection once they have been consumed by the stream, so unlike
@@ -714,19 +696,51 @@ public class Query
 	}
 
 	/**
-	 * Return an Iterator implementation to fetch one PO at a time. This implementation is equivalent to
-	 * <tt>stream().iterator()</tt> and has similar performance benefits compared with {@link #list()}.
-	 * Useful if the downstream API requires an <code>Iterator</code>; otherwise the additional
-	 * of the {@link #stream()} interface is likely to be more useful.
-	 *
+	 * Return an Iterator implementation to fetch one PO at a time. The implementation first retrieve
+	 * all IDS that match the query criteria and issue sql query to fetch the PO when caller want to
+	 * fetch the next PO. This minimize memory usage but it is slower than the list method.
 	 * @return Iterator
 	 * @throws DBException 
-	 * @see #stream()
 	 */
-	@SuppressWarnings("unchecked")
 	public <T extends PO> Iterator<T> iterate() throws DBException
 	{
-		return (Iterator<T>) stream().iterator();
+		String[] keys = table.getKeyColumns();
+		StringBuilder sqlBuffer = new StringBuilder(" SELECT ");
+		for (int i = 0; i < keys.length; i++) {
+			if (i > 0)
+				sqlBuffer.append(", ");
+			if (!joinClauseList.isEmpty())
+				sqlBuffer.append(table.getTableName()).append(".");
+			sqlBuffer.append(keys[i]);
+		}
+		sqlBuffer.append(" FROM ").append(table.getTableName());
+		String sql = buildSQL(sqlBuffer, true);
+		
+		PreparedStatement pstmt = null;
+		ResultSet rs = null;
+		List<Object[]> idList = new ArrayList<Object[]>();
+		try
+		{
+			pstmt = DB.prepareStatement (sql, trxName);
+			rs = createResultSet(pstmt);
+			while (rs.next ())
+			{
+				Object[] ids = new Object[keys.length];
+				for (int i = 0; i < ids.length; i++) {
+					ids[i] = rs.getObject(i+1);
+				}
+				idList.add(ids);
+			}
+		}
+		catch (SQLException e)
+		{
+			log.log(Level.SEVERE, sql, e);
+			throw new DBException(e, sql);
+		} finally {
+			DB.close(rs, pstmt);
+			rs = null; pstmt = null;
+		}
+		return new POIterator<T>(table, idList, trxName);
 	}
 	
 	/**
diff --git a/org.idempiere.test/src/org/idempiere/test/base/QueryTest.java b/org.idempiere.test/src/org/idempiere/test/base/QueryTest.java
index 2ad7ae37af..5ff9eaf6d4 100644
--- a/org.idempiere.test/src/org/idempiere/test/base/QueryTest.java
+++ b/org.idempiere.test/src/org/idempiere/test/base/QueryTest.java
@@ -104,31 +104,6 @@ public class QueryTest extends AbstractTestCase {
 		softly.assertThat(stream.map(MTable::getTableName)).containsExactly("C_Invoice", "M_InOut");
 	}
 	
-	@Test
-	public void testIterable() throws Exception {
-		Iterable<MTable> query = new Query(Env.getCtx(), "AD_Table", "TableName IN (?,?)", getTrxName())
-									.setParameters("C_Invoice", "M_InOut")
-									.setOrderBy("TableName")
-									.iterable();
-		int i = 0;
-		for (MTable t : query) {
-			if (i == 0)
-			{
-				softly.assertThat(t.getTableName()).as("element 0").isEqualTo("C_Invoice");
-			}
-			else if (i == 1)
-			{
-				softly.assertThat(t.getTableName()).as("element 1").isEqualTo("M_InOut");
-			}
-			else
-			{
-				softly.fail("More objects retrieved than expected: " + t.get_TableName());
-				break;
-			}
-			i++;
-		}
-	}
-	
 	@Test
 	public void testScroll() throws Exception
 	{