IDEMPIERE-5147 Improve Query.first() methods - performance (#1124)

* IDEMPIERE-5147  Improve Query.first() methods - performance

* IDEMPIERE-5147 Query Fix move buildSQL

* IDEMPIERE-5147 Query Fix sql value.

* IDEMPIERE-5147 DB paging Optimalizayion Patch

* IDEMPIERE-5147 Query test fix

org.adempiere.base/src/org/compiere/model/Query.java

@@ -328,12 +328,19 @@ public class Query
 	public <T extends PO> T first() throws DBException
 	{
 		T po = null;
-		String sql = buildSQL(null, true);
+		
+		int oldPageSize = this.pageSize;
+		if(DB.getDatabase().isPagingSupported())
+			setPageSize(1);	// Limit to One record
+		
+		String sql = null;
 		
 		PreparedStatement pstmt = null;
 		ResultSet rs = null;
 		try
 		{
+			sql = buildSQL(null, true);
+			
 			pstmt = DB.prepareStatement (sql, trxName);
 			rs = createResultSet(pstmt);
 			if (rs.next ())
@@ -348,6 +355,7 @@ public class Query
 		} finally {
 			DB.close(rs, pstmt);
 			rs = null; pstmt = null;
+			setPageSize(oldPageSize);
 		}
 		return po;
 	}
@@ -363,12 +371,19 @@ public class Query
 	public <T extends PO> T firstOnly() throws DBException
 	{
 		T po = null;
-		String sql = buildSQL(null, true);
+		
+		int oldPageSize = this.pageSize;
+		if(DB.getDatabase().isPagingSupported())
+			setPageSize(2);	// Limit to 2 Records
+		
+		String sql = null;
 		
 		PreparedStatement pstmt = null;
 		ResultSet rs = null;
 		try
 		{
+			sql = buildSQL(null, true);
+			
 			pstmt = DB.prepareStatement (sql, trxName);
 			rs = createResultSet(pstmt);
 			if (rs.next())
@@ -389,6 +404,7 @@ public class Query
 		{
 			DB.close(rs, pstmt);
 			rs = null; pstmt = null;
+			setPageSize(oldPageSize);
 		}
 		return po;
 	}
@@ -427,13 +443,20 @@ public class Query
 			selectClause.append(table.getTableName()).append(".");
 		selectClause.append(keys[0]);
 		selectClause.append(" FROM ").append(table.getTableName());
-		String sql = buildSQL(selectClause, true);
+		
+		int oldPageSize = this.pageSize;
+		if(DB.getDatabase().isPagingSupported())
+			setPageSize(assumeOnlyOneResult ? 2 : 1);
+
+		String sql = null;
 		
 		int id = -1;
 		PreparedStatement pstmt = null;
 		ResultSet rs = null;
 		try
 		{
+			sql = buildSQL(selectClause, true);
+			
 			pstmt = DB.prepareStatement(sql, trxName);
 			rs = createResultSet(pstmt);
 			if (rs.next())
@@ -453,6 +476,7 @@ public class Query
 		{
 			DB.close(rs, pstmt);
 			rs = null; pstmt = null;
+			setPageSize(oldPageSize);
 		}
 		//
 		return id;

org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java

@@ -1209,18 +1209,24 @@ public class DB_Oracle implements AdempiereDatabase
 		return true;
 	}
 
+	/**
+	 * Implemented using the fetch first and offset feature. use 1 base index for start and end parameter
+	 * @param sql
+	 * @param start
+	 * @param end
+	 */
 	public String addPagingSQL(String sql, int start, int end) {
-		StringBuilder newSql = new StringBuilder("select * from (")
+		StringBuilder newSql = new StringBuilder(sql);
-				.append("   select tb.*, ROWNUM oracle_native_rownum_ from (")
+		if (start > 1) {
-				.append(sql)
+			newSql.append(" OFFSET ")
-				.append(") tb) where oracle_native_rownum_ >= ")
+				.append((start - 1))
-				.append(start);
+				.append( " ROWS");
-		if (end > 0) {
+		}
-			newSql.append(" AND oracle_native_rownum_ <= ")
+		if (end > 0) {
-				.append(end);
+			newSql.append(" FETCH FIRST ")
+				.append(( end - start + 1 ))
+				.append(" ROWS ONLY");
 		}
-		newSql.append(" order by oracle_native_rownum_");
-
 		return newSql.toString();
 	}
 

org.compiere.db.postgresql.provider/src/org/compiere/db/DB_PostgreSQL.java

@@ -1043,21 +1043,23 @@ public class DB_PostgreSQL implements AdempiereDatabase
 	}
 
 	/**
-	 * Implemented using the limit and offset feature. use 1 base index for start and end parameter
+	 * Implemented using the fetch first and offset feature. use 1 base index for start and end parameter
 	 * @param sql
 	 * @param start
 	 * @param end
 	 */
 	public String addPagingSQL(String sql, int start, int end) {
 		StringBuilder newSql = new StringBuilder(sql);
-		if (end > 0) {
+		if (start > 1) {
-			newSql.append(" ")
+			newSql.append(" OFFSET ")
-				.append(markNativeKeyword("LIMIT "))
+				.append((start - 1))
-				.append(( end - start + 1 ));
+				.append( " ROWS");
+		}
+		if (end > 0) {
+			newSql.append(" FETCH FIRST ")
+				.append(( end - start + 1 ))
+				.append(" ROWS ONLY");
 		}
-		newSql.append(" ")
-			.append(markNativeKeyword("OFFSET "))
-			.append((start - 1));
 		return newSql.toString();
 	}
 

org.idempiere.test/src/org/idempiere/test/base/QueryTest.java

@@ -231,6 +231,40 @@ public class QueryTest extends AbstractTestCase {
 		});
 	}
 
+	@Test
+	public void testPaging() {
+		DB.executeUpdateEx("DELETE FROM Test WHERE Name LIKE 'QueryTest%'", getTrxName());
+		for (int i=101; i<=130; i++) {
+			PO testPo = new MTest(Env.getCtx(), "QueryTest", i);
+			testPo.save();
+		}
+		Query query = new Query(Env.getCtx(), MTest.Table_Name, "Name LIKE 'QueryTest%'", getTrxName())
+				.setClient_ID()
+				.setOrderBy(MTest.COLUMNNAME_T_Integer);
+		List<MTest> list;
+		list = query.list();
+		assertEquals(list.size(), 30, "Query list without paging brought more records than expected");
+		MTest test = query.first();
+		assertEquals(test.getT_Integer(), 101, "Query first get wrong record");
+		query.setPageSize(10);
+		list = query.list();
+		assertEquals(list.size(), 10, "Query list with paging no skip brought more records than expected");
+		assertEquals(list.get(0).getT_Integer(), 101, "Query list with paging no skip get wrong first record");
+		query.setRecordstoSkip(10);
+		list = query.list();
+		assertEquals(list.size(), 10, "Query list with paging and skip brought more records than expected");
+		assertEquals(list.get(0).getT_Integer(), 111, "Query list with paging and skip get wrong first record");
+		query.setRecordstoSkip(25);
+		list = query.list();
+		assertEquals(list.size(), 5, "Query list last page with paging and skipbrought more records than expected");
+		assertEquals(list.get(0).getT_Integer(), 126, "Query list last page with paging and skip get wrong first record");
+		query.setPageSize(0);
+		query.setRecordstoSkip(10);
+		list = query.list();
+		assertEquals(list.size(), 20, "Query list with skip without paging brought more records than expected");
+		assertEquals(list.get(0).getT_Integer(), 111, "Query list with skip without paging get wrong first record");
+	}
+
 	@Test
 	public void testSetClient_ID() throws Exception
 	{