HHH-12729 : Binary and behavioral incompatibilities of org.hibernate.Query.getFirstResult(), setFirstResult(), getMaxResults(), setMaxResults()

hibernate-core/src/main/java/org/hibernate/Query.java

@@ -22,6 +22,7 @@ import javax.persistence.Parameter;
 import javax.persistence.TemporalType;
 import javax.persistence.TypedQuery;
 
+import org.hibernate.engine.spi.RowSelection;
 import org.hibernate.query.CommonQueryContract;
 import org.hibernate.query.ParameterMetadata;
 import org.hibernate.query.QueryParameter;
@@ -64,6 +65,106 @@ public interface Query<R> extends TypedQuery<R>, CommonQueryContract {
 	 */
 	String getQueryString();
 
+	/**
+	 * "QueryOptions" is a better name, I think, than "RowSelection" -> 6.0
+	 *
+	 * @todo 6.0 rename RowSelection to QueryOptions
+	 *
+	 * @return Return the encapsulation of this query's options, which includes access to
+	 * firstRow, maxRows, timeout and fetchSize.   Important because this gives access to
+	 * those values in their Integer form rather than the primitive form (int) required by JPA.
+	 */
+	RowSelection getQueryOptions();
+
+	/**
+	 * The position of the first query result to be retrieved, previously set by
+	 * {@link #setFirstResult(int)} or {@link #setHibernateFirstResult(int)}.
+	 * <p/>
+	 * If the value was not initialized by {@link #setFirstResult(int)} or
+	 * {@link #setHibernateFirstResult(int)}, then {@code null} is returned, resulting
+	 * in pagination starting from position 0.
+	 * <p/>
+	 * If {@link #setHibernateFirstResult(int)} was called with a negative value, then 0
+	 * is returned.
+	 *
+	 * @return the position of the first query result, or {@code null} if uninitialized.
+	 *
+	 * @see #setFirstResult(int)
+	 * @see #setHibernateFirstResult(int)
+	 *
+	 * @deprecated {@link #setFirstResult(int)} should be used instead.
+	 */
+	@Deprecated
+	default Integer getHibernateFirstResult() {
+		return getQueryOptions().getFirstRow();
+	}
+
+	/**
+	 * Set the position of the first query result to be retrieved. A negative value will
+	 * result in pagination starting from position 0.
+	 *
+	 * @param firstRow - the position of the first query result
+	 * @return {@code this}, for method chaining
+	 *
+	 * @deprecated {@link #setFirstResult(int)} should be used instead.
+	 */
+	@Deprecated
+	default Query setHibernateFirstResult(int firstRow) {
+		if ( firstRow < 0 ) {
+			getQueryOptions().setFirstRow( 0 );
+		}
+		else {
+			getQueryOptions().setFirstRow( firstRow );
+		}
+		return this;
+	}
+
+	/**
+	 * The maximum number of query results to be retrieved, previously set by
+	 * {@link #setMaxResults(int)} or {@link #setHibernateMaxResults(int)}.
+	 * <p/>
+	 * If the value was not initialized by {@link #setMaxResults(int)} or
+	 * {@link #setHibernateMaxResults(int)}, then {@code null} is returned
+	 * <p/>
+	 * If {@link #setHibernateMaxResults(int)} was called with a value less than
+	 * or equal to 0, the value is considered to be uninitialized, and {@code null}
+	 * is returned, resulting in no limit on the number of results.
+	 *
+	 * @return the maximum number of query results, or {@code null} if uninitialized.
+	 *
+	 * @see #setFirstResult(int)
+	 * @see #setHibernateFirstResult(int)
+	 *
+	 * @deprecated {@link #setFirstResult(int)} should be used instead.
+	 */
+	@Deprecated
+	default Integer getHibernateMaxResults() {
+		return getQueryOptions().getMaxRows();
+	}
+
+	/**
+	 * Set the maximum number of query results to be retrieved. A value less than
+	 * or equal to 0 is considered uninitialized, resulting in no limit on the number
+	 * of results.
+	 *
+	 * @param maxResults - the maximum number of query results
+	 * @return {@code this}, for method chaining
+	 *
+	 * @deprecated {@link #setMaxResults(int)} should be used instead.
+	 */
+	@Deprecated
+	default Query setHibernateMaxResults(int maxResults) {
+		// maxResults <= 0 is the same as uninitialized (with no limit),
+		if ( maxResults <= 0 ) {
+			// treat zero and negatives specifically as meaning no limit...
+			getQueryOptions().setMaxRows( null );
+		}
+		else {
+			getQueryOptions().setMaxRows( maxResults );
+		}
+		return this;
+	}
+
 	/**
 	 * Obtain the FlushMode in effect for this query.  By default, the query inherits the FlushMode of the Session
 	 * from which it originates.

hibernate-core/src/main/java/org/hibernate/query/Query.java

@@ -75,17 +75,6 @@ public interface Query<R> extends TypedQuery<R>, org.hibernate.Query<R>, CommonQ
 	 */
 	QueryProducer getProducer();
 
-	/**
-	 * "QueryOptions" is a better name, I think, than "RowSelection" -> 6.0
-	 *
-	 * @todo 6.0 rename RowSelection to QueryOptions
-	 *
-	 * @return Return the encapsulation of this query's options, which includes access to
-	 * firstRow, maxRows, timeout and fetchSize.   Important because this gives access to
-	 * those values in their Integer form rather than the primitive form (int) required by JPA.
-	 */
-	RowSelection getQueryOptions();
-
 	Optional<R> uniqueResultOptional();
 
 	/**

hibernate-core/src/test/java/org/hibernate/test/hql/HibernateFirstResultMaxResultsTest.java

@@ -0,0 +1,145 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.test.hql;
+
+import java.util.List;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+import org.hibernate.Query;
+import org.hibernate.Session;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+/**
+ * @author Gail Badner
+ */
+@TestForIssue( jiraKey = "HHH-12729")
+public class HibernateFirstResultMaxResultsTest extends BaseNonConfigCoreFunctionalTestCase {
+
+	protected Class[] getAnnotatedClasses() {
+		return new Class[] { Employee.class };
+	}
+
+	@Test
+	public void testFirstResult() {
+		doInHibernate(
+				this::sessionFactory,
+				session -> {
+
+					Query query = session.createQuery( "from Employee" );
+
+					// not initialized yet
+					assertNull( query.getHibernateFirstResult() );
+
+					// the following is special case; when initialized to -1, getHibernateFirstResult returns 0
+					assertEquals( Integer.valueOf( 0 ), query.setHibernateFirstResult( -1 ).getHibernateFirstResult() );
+
+					assertEquals( Integer.valueOf( 0 ), query.setHibernateFirstResult( 0 ).getHibernateFirstResult() );
+					assertEquals( Integer.valueOf( 1 ), query.setHibernateFirstResult( 1 ).getHibernateFirstResult() );
+
+					assertEquals( Integer.valueOf( 10 ), query.setFirstResult( 10 ).getHibernateFirstResult() );
+				}
+		);
+	}
+
+	@Test
+	public void testMaxResults() {
+		doInHibernate(
+				this::sessionFactory,
+				session -> {
+
+					Query query = session.createQuery( "from Employee" );
+
+					// not initialized yet
+					assertNull( query.getHibernateMaxResults() );
+
+					// values <= 0 are considered uninitialized;
+					assertNull( query.setHibernateMaxResults( -1 ).getHibernateMaxResults() );
+					assertNull( query.setHibernateMaxResults( 0 ).getHibernateMaxResults() );
+
+					assertEquals( Integer.valueOf( 1 ), query.setHibernateMaxResults( 1 ).getHibernateMaxResults() );
+
+					assertEquals( Integer.valueOf( 0 ), query.setMaxResults( 0 ).getHibernateMaxResults() );
+
+					assertEquals( Integer.valueOf( 2 ), query.setMaxResults( 2 ).getHibernateMaxResults() );
+				}
+		);
+	}
+
+	@Test
+	public void testPagination() {
+		doInHibernate(
+				this::sessionFactory,
+				session -> {
+					for ( int i = 0; i < 5; i++ ) {
+						session.persist( new Employee( i ) );
+					}
+				}
+		);
+
+		final String query = "from Employee order by id";
+		checkResults( executeQuery( query, null, null ), 0, 4 );
+		checkResults( executeQuery( query, 0, null ), 0, 4 );
+		checkResults( executeQuery( query, -1, null ), 0, 4 );
+		checkResults( executeQuery( query, null, 0 ), 0, 4 );
+		checkResults( executeQuery( query, null, -1 ), 0, 4 );
+		checkResults( executeQuery( query, null, 2 ), 0, 1 );
+		checkResults( executeQuery( query, -1, 0 ), 0, 4 );
+		checkResults( executeQuery( query, -1, 3 ), 0, 2 );
+		checkResults( executeQuery( query, 1, null ), 1, 4 );
+		checkResults( executeQuery( query, 1, 0 ), 1, 4 );
+		checkResults( executeQuery( query, 1, -1 ), 1, 4 );
+		checkResults( executeQuery( query, 1, 1 ), 1, 1 );
+	}
+
+	public List executeQuery(String queryString, Integer firstResult, Integer maxResults) {
+		return doInHibernate(
+				this::sessionFactory,
+				session -> {
+					Query query = session.createQuery( queryString );
+					if ( firstResult != null ) {
+						query.setHibernateFirstResult( firstResult );
+					}
+					if ( maxResults != null ) {
+						query.setHibernateMaxResults( maxResults );
+					}
+					return query.list();
+				}
+		);
+	}
+
+	private void checkResults( List results, int firstIdExpected, int lastIdExpected ) {
+		int resultIndex = 0;
+		for( int i = firstIdExpected ; i <= lastIdExpected ; i++, resultIndex++ ) {
+			assertEquals( i, ( (Employee) results.get( resultIndex ) ).id );
+		}
+	}
+
+	@Entity(name = "Employee")
+	public static class Employee {
+		@Id
+		private long id;
+
+		private String name;
+
+		public Employee() {
+		}
+
+		public Employee(long id) {
+			this.id = id;
+		}
+	}
+
+}