HHH-16384 - Fix NPE in AbstractSelectionQuery.setFirstResult

Included check for that in PaginationTest
Moved PaginationTest to Junit5

Signed-off-by: Jan Schatteman <jschatte@redhat.com>

hibernate-core/src/main/java/org/hibernate/query/spi/AbstractQuery.java

@@ -155,14 +155,7 @@ public abstract class AbstractQuery<R>
 
 	@Override
 	public QueryImplementor<R> setFirstResult(int startPosition) {
-		getSession().checkOpen();
+		super.setFirstResult( startPosition );
-
-		if ( startPosition < 0 ) {
-			throw new IllegalArgumentException( "first-result value cannot be negative : " + startPosition );
-		}
-
-		getQueryOptions().getLimit().setFirstRow( startPosition );
-
 		return this;
 	}
 

hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java

@@ -554,7 +554,15 @@ public abstract class AbstractSelectionQuery<R>
 
 	@Override
 	public SelectionQuery<R> setFirstResult(int startPosition) {
-		return null;
+		getSession().checkOpen();
+
+		if ( startPosition < 0 ) {
+			throw new IllegalArgumentException( "first-result value cannot be negative : " + startPosition );
+		}
+
+		getQueryOptions().getLimit().setFirstRow( startPosition );
+
+		return this;
 	}
 
 	@Override

hibernate-core/src/test/java/org/hibernate/orm/test/pagination/PaginationTest.java

@@ -7,6 +7,7 @@
 package org.hibernate.orm.test.pagination;
 
 import java.math.BigDecimal;
+import java.math.RoundingMode;
 import java.util.List;
 
 import jakarta.persistence.criteria.CriteriaBuilder;
@@ -16,39 +17,32 @@ import jakarta.persistence.criteria.Root;
 import org.hibernate.query.NativeQuery;
 import org.hibernate.query.Query;
 import org.hibernate.Session;
+import org.hibernate.query.SelectionQuery;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
-import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
+import static org.junit.jupiter.api.Assertions.assertEquals;
-import org.junit.After;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-import org.junit.Before;
-import org.junit.Test;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
 /**
  * @author Gavin King
  */
-public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
+@DomainModel(
+		xmlMappings = {
+				"org/hibernate/orm/test/pagination/DataPoint.hbm.xml"
+		}
+)
+@SessionFactory
+public class PaginationTest {
 	public static final int NUMBER_OF_TEST_ROWS = 100;
 
-	@Override
-	protected String getBaseForMappings() {
-		return "org/hibernate/orm/test/";
-	}
-
-	@Override
-	public String[] getMappings() {
-		return new String[] { "pagination/DataPoint.hbm.xml" };
-	}
-
-	@Override
-	public String getCacheConcurrencyStrategy() {
-		return null;
-	}
-
 	@Test
-	public void testLimit() {
+	public void testLimit(SessionFactoryScope scope) {
-		inTransaction(
+		scope.inTransaction(
 				session -> {
 					int count;
 
@@ -58,6 +52,12 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 							.size();
 					assertEquals( 5, count );
 
+					count = generateBaseSelectionQuery( session )
+							.setMaxResults( 5 )
+							.list()
+							.size();
+					assertEquals( 5, count );
+
 					count = generateBaseQuery( session )
 							.setMaxResults( 18 )
 							.list()
@@ -74,8 +74,8 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 	}
 
 	@Test
-	public void testOffset() {
+	public void testOffset(SessionFactoryScope scope) {
-		inTransaction(
+		scope.inTransaction(
 				session -> {
 					List result;
 
@@ -90,18 +90,29 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 					DataPoint firstDataPointCriteria = (DataPoint) result.get( 0 );
 
 					assertEquals(
-							"The first entry should be the same in HQL and Criteria",
 							firstDataPointHQL,
-							firstDataPointHQL
+							firstDataPointHQL,
+							"The first entry should be the same in HQL and Criteria"
+					);
+					assertEquals( 3, firstDataPointCriteria.getSequence(), "Wrong first result" );
+
+					result = generateBaseSelectionQuery( session )
+							.setFirstResult( 3 )
+							.list();
+					firstDataPointHQL = (DataPoint) result.get( 0 );
+
+					assertEquals(
+							firstDataPointHQL,
+							firstDataPointHQL,
+							"The first entry should be the same in HQL and Criteria"
 					);
-					assertEquals( "Wrong first result", 3, firstDataPointCriteria.getSequence() );
 				}
 		);
 	}
 
 	@Test
-	public void testLimitOffset() {
+	public void testLimitOffset(SessionFactoryScope scope) {
-		inTransaction(
+		scope.inTransaction(
 				session -> {
 					List result;
 
@@ -113,6 +124,14 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 					assertEquals( 0, ( (DataPoint) result.get( 0 ) ).getSequence() );
 					assertEquals( 1, ( (DataPoint) result.get( 1 ) ).getSequence() );
 
+					result = generateBaseSelectionQuery( session )
+							.setFirstResult( 0 )
+							.setMaxResults( 20 )
+							.list();
+					assertEquals( 20, result.size() );
+					assertEquals( 0, ( (DataPoint) result.get( 0 ) ).getSequence() );
+					assertEquals( 1, ( (DataPoint) result.get( 1 ) ).getSequence() );
+
 					result = generateBaseQuery( session )
 							.setFirstResult( 1 )
 							.setMaxResults( 20 )
@@ -128,7 +147,7 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 					assertEquals( 1, result.size() );
 					assertEquals( 99, ( (DataPoint) result.get( 0 ) ).getSequence() );
 
-					result = session.createQuery( "select distinct description from DataPoint order by description" )
+					result = session.createQuery( "select distinct description from DataPoint order by description", String.class )
 							.setFirstResult( 2 )
 							.setMaxResults( 3 )
 							.list();
@@ -138,7 +157,7 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 					assertEquals( "Description: 4", result.get( 2 ) );
 
 					result = session.createNativeQuery(
-							"select description, xval, yval from DataPoint order by xval, yval" )
+							"select description, xval, yval from DataPoint order by xval, yval", Object[].class )
 							.setFirstResult( 2 )
 							.setMaxResults( 5 )
 							.list();
@@ -146,7 +165,7 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 					Object[] row = (Object[]) result.get( 0 );
 					assertTrue( row[0] instanceof String );
 
-					result = session.createNativeQuery( "select * from DataPoint order by xval, yval" )
+					result = session.createNativeQuery( "select * from DataPoint order by xval, yval", Object[].class )
 							.setFirstResult( 2 )
 							.setMaxResults( 5 )
 							.list();
@@ -157,50 +176,49 @@ public class PaginationTest extends BaseNonConfigCoreFunctionalTestCase {
 	}
 
 	private Query generateBaseHQLQuery(Session session) {
-		return session.createQuery( "select dp from DataPoint dp order by dp.sequence" );
+		return session.createQuery( "select dp from DataPoint dp order by dp.sequence", DataPoint.class );
+	}
+
+	private SelectionQuery generateBaseSelectionQuery(Session session) {
+		return session.createSelectionQuery( "select dp from DataPoint dp order by dp.sequence", DataPoint.class );
 	}
 
 	private Query generateBaseQuery(Session session) {
 		CriteriaBuilder criteriaBuilder = session.getCriteriaBuilder();
 		CriteriaQuery<DataPoint> criteria = criteriaBuilder.createQuery( DataPoint.class );
 		Root<DataPoint> root = criteria.from( DataPoint.class );
-		return session.createQuery( criteria.orderBy( criteriaBuilder.asc( root ) ) );
+		return session.createQuery( criteria.orderBy( criteriaBuilder.asc( root.get( "sequence" ) ) ) );
-//		return session.createCriteria( DataPoint.class )
-//				.addOrder( Order.asc( "sequence" ) );
 	}
 
 	private NativeQuery generateBaseSQLQuery(Session session) {
-		return session.createNativeQuery( "select id, seqval, xval, yval, description from DataPoint order by seqval" )
+		return session.createNativeQuery( "select id, seqval, xval, yval, description from DataPoint order by seqval", Object[].class )
 				.addEntity( DataPoint.class );
 	}
 
-	@Before
+	@BeforeEach
-	public void prepareTestData() {
+	public void prepareTestData(SessionFactoryScope scope) {
-		inTransaction(
+		scope.inTransaction(
 				session -> {
 					for ( int i = 0; i < NUMBER_OF_TEST_ROWS; i++ ) {
 						DataPoint dataPoint = new DataPoint();
 						dataPoint.setSequence( i );
-						dataPoint.setDescription( "data point #" + i );
+						BigDecimal x = new BigDecimal( i * 0.1d ).setScale( 19, RoundingMode.DOWN );
-						BigDecimal x = new BigDecimal( i * 0.1d ).setScale( 19, BigDecimal.ROUND_DOWN );
 						dataPoint.setX( x );
 						dataPoint.setY( new BigDecimal( Math.cos( x.doubleValue() ) ).setScale(
 								19,
-								BigDecimal.ROUND_DOWN
+								RoundingMode.DOWN
 						) );
 						dataPoint.setDescription( "Description: " + i % 5 );
-						session.save( dataPoint );
+						session.persist( dataPoint );
 					}
 				}
 		);
 	}
 
-	@After
+	@AfterEach
-	public void cleanupTestData() {
+	public void cleanupTestData(SessionFactoryScope scope) {
-		Session session = openSession();
+		scope.inTransaction(
-		session.beginTransaction();
+				session -> session.createMutationQuery( "delete from DataPoint" ).executeUpdate()
-		session.createQuery( "delete DataPoint" ).executeUpdate();
+		);
-		session.getTransaction().commit();
-		session.close();
 	}
 }