DATAJPA-1255 - Fixed application of JPA pagination to only queries that don't use #pageable explicitly.

Query.setMaximumResults(…) and Query.setFirstResult(…) now only get applied when the underlying query does not mention #pageable.

Before we called it unconditionally which caused the JPA implementation to apply it’s transformation to SQL statements that already were correctly doing pagination.

Interestingly the existing test cases didn’t fail although the executed SQL was pretty bogus.
Therefore the tests got changed a little to make them more strict.

Original pull request: #247.

src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java

@@ -83,4 +83,11 @@ interface DeclaredQuery {
 	 * @return a new {@literal DeclaredQuery} instance.
 	 */
 	DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection);
+
+	/**
+	 * @return whether paging is implemented in the query itself, e.g. using SpEL expressions.
+	 */
+	default boolean implementsPaging() {
+		return false;
+	}
 }

src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java

@@ -37,20 +37,37 @@ public class ParameterBinder {
 
 	private final JpaParameters parameters;
 	private final Iterable<QueryParameterSetter> parameterSetters;
+	private final boolean useJpaForPaging;
 
 	/**
 	 * Creates a new {@link ParameterBinder} for the given {@link JpaParameters} and {@link QueryParameterSetter}s.
 	 *
 	 * @param parameters must not be {@literal null}.
 	 * @param parameterSetters must not be {@literal null}.
+	 * @param useJpaForPaging determines whether {@link Query#setFirstResult(int)} and {@link Query#setMaxResults(int)}
+	 *          shall be used for paging.
 	 */
-	public ParameterBinder(JpaParameters parameters, Iterable<QueryParameterSetter> parameterSetters) {
+	public ParameterBinder(JpaParameters parameters, Iterable<QueryParameterSetter> parameterSetters,
+			boolean useJpaForPaging) {
 
 		Assert.notNull(parameters, "JpaParameters must not be null!");
 		Assert.notNull(parameterSetters, "Parameter setters must not be null!");
 
 		this.parameters = parameters;
 		this.parameterSetters = parameterSetters;
+		this.useJpaForPaging = useJpaForPaging;
+	}
+
+	/**
+	 * Only for backward compatibility.
+	 *
+	 * @param parameters must not be {@literal null}.
+	 * @param parameterSetters must not be {@literal null}.
+	 * @deprecated use three argument constructor instead}
+	 */
+	@Deprecated
+	public ParameterBinder(JpaParameters parameters, Iterable<QueryParameterSetter> parameterSetters) {
+		this(parameters, parameterSetters, true);
 	}
 
 	public <T extends Query> T bind(T jpaQuery, Object[] values) {
@@ -78,7 +95,7 @@ public class ParameterBinder {
 
 		Query result = bind(query, values);
 
-		if (!parameters.hasPageableParameter() || accessor.getPageable().isUnpaged()) {
+		if (!useJpaForPaging || !parameters.hasPageableParameter() || accessor.getPageable().isUnpaged()) {
 			return result;
 		}
 

src/main/java/org/springframework/data/jpa/repository/query/ParameterBinderFactory.java

@@ -52,7 +52,7 @@ class ParameterBinderFactory {
 		QueryParameterSetterFactory setterFactory = QueryParameterSetterFactory.basic(parameters);
 		List<ParameterBinding> bindings = getBindings(parameters);
 
-		return new ParameterBinder(parameters, createSetters(bindings, setterFactory));
+		return new ParameterBinder(parameters, createSetters(bindings, setterFactory), true);
 	}
 
 	/**
@@ -72,7 +72,7 @@ class ParameterBinderFactory {
 		QueryParameterSetterFactory setterFactory = QueryParameterSetterFactory.forCriteriaQuery(parameters, metadata);
 		List<ParameterBinding> bindings = getBindings(parameters);
 
-		return new ParameterBinder(parameters, createSetters(bindings, setterFactory));
+		return new ParameterBinder(parameters, createSetters(bindings, setterFactory), true);
 	}
 
 	/**
@@ -100,7 +100,7 @@ class ParameterBinderFactory {
 				evaluationContextProvider, parameters);
 		QueryParameterSetterFactory basicSetterFactory = QueryParameterSetterFactory.basic(parameters);
 
-		return new ParameterBinder(parameters, createSetters(bindings, query, expressionSetterFactory, basicSetterFactory));
+		return new ParameterBinder(parameters, createSetters(bindings, query, expressionSetterFactory, basicSetterFactory), !query.implementsPaging());
 	}
 
 	private static List<ParameterBinding> getBindings(JpaParameters parameters) {

src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java

@@ -53,6 +53,7 @@ class StringQuery implements DeclaredQuery {
 	private final List<ParameterBinding> bindings;
 	private final @Nullable String alias;
 	private final boolean hasConstructorExpression;
+	private final boolean containsPageableInSpel;
 
 	/**
 	 * Creates a new {@link StringQuery} from the given JPQL query.
@@ -64,6 +65,7 @@ class StringQuery implements DeclaredQuery {
 		Assert.hasText(query, "Query must not be null or empty!");
 
 		this.bindings = new ArrayList<>();
+		this.containsPageableInSpel = query.contains("#pageable");
 		this.query = ParameterBindingParser.INSTANCE.parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(query,
 				this.bindings);
 
@@ -102,6 +104,11 @@ class StringQuery implements DeclaredQuery {
 				.of(countQuery != null ? countQuery : QueryUtils.createCountQueryFor(query, countQueryProjection));
 	}
 
+	@Override
+	public boolean implementsPaging() {
+		return containsPageableInSpel;
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.jpa.repository.query.DeclaredQuery#getQueryString()

src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java

@@ -1524,13 +1524,17 @@ public class UserRepositoryTests {
 
 		flushTestUsers();
 
-		Page<User> users = repository.findUsersInNativeQueryWithPagination(PageRequest.of(0, 2));
+		Page<User> users = repository.findUsersInNativeQueryWithPagination(PageRequest.of(0, 3));
 
-		assertThat(users.getContent()).hasSize(2).containsExactly(firstUser, secondUser);
+		SoftAssertions softly = new SoftAssertions();
 
-		users = repository.findUsersInNativeQueryWithPagination(PageRequest.of(1, 2));
+		softly.assertThat(users.getContent()).extracting(User::getFirstname).containsExactly("Dave", "Joachim", "kevin");
 
-		assertThat(users.getContent()).hasSize(2).containsExactly(thirdUser, fourthUser);
+		users = repository.findUsersInNativeQueryWithPagination(PageRequest.of(1, 3));
+
+		softly.assertThat(users.getContent()).extracting(User::getFirstname).containsExactly("Oliver");
+
+		softly.assertAll();
 	}
 
 	@Test // DATAJPA-1140

src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java

@@ -425,7 +425,11 @@ public interface UserRepository
 
 	// DATAJPA-564
 	@Query(
-			value = "select * from (select rownum() as RN, u.* from SD_User u) where RN between ?#{ #pageable.offset -1} and ?#{#pageable.offset + #pageable.pageSize}",
+			value = "select * from (" +
+					"select u.*, rownum() as RN from (" +
+					"select * from SD_User ORDER BY ucase(firstname)" +
+					") u" +
+					") where RN between ?#{ #pageable.offset +1 } and ?#{#pageable.offset + #pageable.pageSize}",
 			countQuery = "select count(u.id) from SD_User u", nativeQuery = true)
 	Page<User> findUsersInNativeQueryWithPagination(Pageable pageable);
 
@@ -534,7 +538,8 @@ public interface UserRepository
 	Page<User> findByNativeNamedQueryWithPageable(Pageable pageable);
 
 	// DATAJPA-928
-	@Query(value = "SELECT firstname FROM SD_User ORDER BY UCASE(firstname)", countQuery = "SELECT count(*) FROM SD_User", nativeQuery = true)
+	@Query(value = "SELECT firstname FROM SD_User ORDER BY UCASE(firstname)", countQuery = "SELECT count(*) FROM SD_User",
+			nativeQuery = true)
 	Page<String> findByNativeQueryWithPageable(@Param("pageable") Pageable pageable);
 
 	// DATAJPA-1273