Spring/spring-data-jpa
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Skip parameter lookup for unused query parameters.

Browse Source 
We now skip binding parameter lookup if the query isn't using named parameters and the parameter is not associated with a name. Also, we check for presence of lookup identifiers to avoid parameter binding that cannot be looked up as they are not used anymore.

This can happen when a declared query uses parameters only in the ORDER BY clause that is truncated during count query derivation. Then the query object reports parameters althtough they are not being used.

We also refined parameter carryover during count query derivation. Previously, we copied all parameters without introspecting their origin. now, we copy only expression parameters to the derived query as count query derivation doesn't have access to expressions as our query parsers require valid JPQL.

Closes #3756
This commit is contained in:
Mark Paluch
2025-01-29 15:07:47 +01:00
parent 9672b30e78
commit 1fc50dd3c9
6 changed files with 118 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java
View File

@@ -245,10 +245,13 @@ abstract class QueryParameterSetterFactory {
BindingIdentifier identifier = mia.identifier();
if (declaredQuery.hasNamedParameter()) {
if (declaredQuery.hasNamedParameter() && identifier.hasName()) {
parameter = findParameterForBinding(parameters, identifier.getName());
} else {
} else if (identifier.hasPosition()) {
parameter = findParameterForBinding(parameters, identifier.getPosition() - 1);
} else {
// this can happen when a query uses parameters in ORDER BY and the COUNT query just needs to drop a binding.
parameter = null;
}
return parameter == null //

20
spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java
View File

@@ -113,9 +113,19 @@ class StringQuery implements DeclaredQuery {
StringQuery stringQuery = new StringQuery(this.queryEnhancer.createCountQueryFor(countQueryProjection), //
this.isNative);
// need to copy expression bindings from the declared to the derived query as JPQL query derivation only sees JPA
// parameter markers and not the original expressions anymore.
if (this.hasParameterBindings() && !this.getParameterBindings().equals(stringQuery.getParameterBindings())) {
stringQuery.getParameterBindings().clear();
stringQuery.getParameterBindings().addAll(this.bindings);
List<ParameterBinding> derivedBindings = stringQuery.getParameterBindings();
for (ParameterBinding binding : bindings) {
if (binding.getOrigin().isExpression() && derivedBindings
.removeIf(it -> !it.getOrigin().isExpression() && it.getIdentifier().equals(binding.getIdentifier()))) {
derivedBindings.add(binding);
}
}
}
return stringQuery;
@@ -235,8 +245,7 @@ class StringQuery implements DeclaredQuery {
greatestParameterIndex = 0;
}
SpelExtractor spelExtractor = createSpelExtractor(query, parametersShouldBeAccessedByIndex,
greatestParameterIndex);
SpelExtractor spelExtractor = createSpelExtractor(query, parametersShouldBeAccessedByIndex, greatestParameterIndex);
String resultingQuery = spelExtractor.getQueryString();
Matcher matcher = PARAMETER_BINDING_PATTERN.matcher(resultingQuery);
@@ -340,8 +349,7 @@ class StringQuery implements DeclaredQuery {
return resultingQuery;
}
private static SpelExtractor createSpelExtractor(String queryWithSpel, boolean parametersShouldBeAccessedByIndex,
int greatestParameterIndex) {
private static SpelExtractor createSpelExtractor(String queryWithSpel, boolean parametersShouldBeAccessedByIndex, int greatestParameterIndex) {
/*
* If parameters need to be bound by index, we bind the synthetic expression parameters starting from position of the greatest discovered index parameter in order to

36
spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java
View File

@@ -3016,22 +3016,36 @@ class UserRepositoryTests {
assertThat(users).extracting(User::getId).containsExactly(expected.getId());
}
@Disabled("ORDER BY CASE appears to be a Hibernate-only feature")
@Test // DATAJPA-1233
@Test // DATAJPA-1233, GH-3756
void handlesCountQueriesWithLessParametersSingleParam() {
// repository.findAllOrderedBySpecialNameSingleParam("Oliver", PageRequest.of(2, 3));
flushTestUsers();
Page<User> result = repository.findAllOrderedByNamedParam("Oliver", PageRequest.of(0, 3));
assertThat(result.getContent()).containsExactly(firstUser, fourthUser, thirdUser);
assertThat(result.getTotalElements()).isEqualTo(4);
result = repository.findAllOrderedByIndexedParam("Oliver", PageRequest.of(0, 3));
assertThat(result.getContent()).containsExactly(firstUser, fourthUser, thirdUser);
assertThat(result.getTotalElements()).isEqualTo(4);
}
@Disabled("ORDER BY CASE appears to be a Hibernate-only feature")
@Test // DATAJPA-1233
@Test // DATAJPA-1233, GH-3756
void handlesCountQueriesWithLessParametersMoreThanOne() {
// repository.findAllOrderedBySpecialNameMultipleParams("Oliver", "x", PageRequest.of(2, 3));
}
@Disabled("ORDER BY CASE appears to be a Hibernate-only feature")
@Test // DATAJPA-1233
void handlesCountQueriesWithLessParametersMoreThanOneIndexed() {
// repository.findAllOrderedBySpecialNameMultipleParamsIndexed("x", "Oliver", PageRequest.of(2, 3));
flushTestUsers();
Page<User> result = repository.findAllOrderedBySpecialNameMultipleParams("Oliver", "x", PageRequest.of(0, 3));
assertThat(result.getContent()).containsExactly(firstUser, fourthUser, thirdUser);
assertThat(result.getTotalElements()).isEqualTo(4);
result = repository.findAllOrderedBySpecialNameMultipleParamsIndexed("x", "Oliver", PageRequest.of(0, 3));
assertThat(result.getContent()).containsExactly(firstUser, fourthUser, thirdUser);
assertThat(result.getTotalElements()).isEqualTo(4);
}
// DATAJPA-928

6
spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ExpressionBasedStringQueryUnitTests.java
View File

@@ -176,7 +176,7 @@ class ExpressionBasedStringQueryUnitTests {
}
@Test
public void doesTemplatingWhenEntityNameSpelIsPresent() {
void doesTemplatingWhenEntityNameSpelIsPresent() {
StringQuery query = new ExpressionBasedStringQuery("select #{#entityName + 'Hallo'} from #{#entityName} u",
metadata, SPEL_PARSER, false);
@@ -185,7 +185,7 @@ class ExpressionBasedStringQueryUnitTests {
}
@Test
public void doesNoTemplatingWhenEntityNameSpelIsNotPresent() {
void doesNoTemplatingWhenEntityNameSpelIsNotPresent() {
StringQuery query = new ExpressionBasedStringQuery("select #{#entityName + 'Hallo'} from User u", metadata,
SPEL_PARSER, false);
@@ -194,7 +194,7 @@ class ExpressionBasedStringQueryUnitTests {
}
@Test
public void doesTemplatingWhenEntityNameSpelIsPresentForBindParameter() {
void doesTemplatingWhenEntityNameSpelIsPresentForBindParameter() {
StringQuery query = new ExpressionBasedStringQuery("select u from #{#entityName} u where name = :#{#something}",
metadata, SPEL_PARSER, false);

50
spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java
View File

@@ -309,6 +309,56 @@ class StringQueryUnitTests {
assertNamedBinding(InParameterBinding.class, "foo_1", bindings.get(1));
}
@Test // GH-3126
void countQueryDerivationRetainsNamedExpressionParameters() {
StringQuery query = new StringQuery(
"select u from User u where foo = :#{bar} ORDER BY CASE WHEN (u.firstname >= :#{name}) THEN 0 ELSE 1 END",
false);
DeclaredQuery countQuery = query.deriveCountQuery(null);
assertThat(countQuery.getParameterBindings()).hasSize(1);
assertThat(countQuery.getParameterBindings()).extracting(ParameterBinding::getOrigin)
.extracting(ParameterOrigin::isExpression).isEqualTo(List.of(true));
query = new StringQuery(
"select u from User u where foo = :#{bar} and bar = :bar ORDER BY CASE WHEN (u.firstname >= :bar) THEN 0 ELSE 1 END",
false);
countQuery = query.deriveCountQuery(null);
assertThat(countQuery.getParameterBindings()).hasSize(2);
assertThat(countQuery.getParameterBindings()) //
.extracting(ParameterBinding::getOrigin) //
.extracting(ParameterOrigin::isExpression).contains(true, false);
}
@Test // GH-3126
void countQueryDerivationRetainsIndexedExpressionParameters() {
StringQuery query = new StringQuery(
"select u from User u where foo = ?#{bar} ORDER BY CASE WHEN (u.firstname >= ?#{name}) THEN 0 ELSE 1 END",
false);
DeclaredQuery countQuery = query.deriveCountQuery(null);
assertThat(countQuery.getParameterBindings()).hasSize(1);
assertThat(countQuery.getParameterBindings()).extracting(ParameterBinding::getOrigin)
.extracting(ParameterOrigin::isExpression).isEqualTo(List.of(true));
query = new StringQuery(
"select u from User u where foo = ?#{bar} and bar = ?1 ORDER BY CASE WHEN (u.firstname >= ?1) THEN 0 ELSE 1 END",
false);
countQuery = query.deriveCountQuery(null);
assertThat(countQuery.getParameterBindings()).hasSize(2);
assertThat(countQuery.getParameterBindings()) //
.extracting(ParameterBinding::getOrigin) //
.extracting(ParameterOrigin::isExpression).contains(true, false);
}
@Test // DATAJPA-461
void detectsMultiplePositionalInParameterBindings() {

45
spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java
View File

@@ -82,9 +82,8 @@ public interface UserRepository extends JpaRepository<User, Integer>, JpaSpecifi
java.util.Optional<User> findById(Integer primaryKey);
/**
* Redeclaration of {@link CrudRepository#deleteById(java.lang.Object)}. to make sure the transaction
* configuration of the original method is considered if the redeclaration does not carry a {@link Transactional}
* annotation.
* Redeclaration of {@link CrudRepository#deleteById(java.lang.Object)}. to make sure the transaction configuration of
* the original method is considered if the redeclaration does not carry a {@link Transactional} annotation.
*/
@Override
void deleteById(Integer id); // DATACMNS-649
@@ -416,7 +415,8 @@ public interface UserRepository extends JpaRepository<User, Integer>, JpaSpecifi
@Query("select u from User u where u.firstname = ?#{[0]} and u.firstname = ?1 and u.lastname like %?#{[1]}% and u.lastname like %?2%")
List<User> findByFirstnameAndLastnameWithSpelExpression(String firstname, String lastname);
@Query(value = "select * from SD_User", countQuery = "select count(1) from SD_User u where u.lastname = :#{#lastname}", nativeQuery = true)
@Query(value = "select * from SD_User",
countQuery = "select count(1) from SD_User u where u.lastname = :#{#lastname}", nativeQuery = true)
Page<User> findByWithSpelParameterOnlyUsedForCountQuery(String lastname, Pageable page);
// DATAJPA-564
@@ -563,26 +563,23 @@ public interface UserRepository extends JpaRepository<User, Integer>, JpaSpecifi
@Query("SELECT u FROM User u where u.firstname >= ?1 and u.lastname = '000:1'")
List<User> queryWithIndexedParameterAndColonFollowedByIntegerInString(String firstname);
/**
* TODO: ORDER BY CASE appears to only with Hibernate. The examples attempting to do this through pure JPQL don't
* appear to work with Hibernate, so we must set them aside until we can implement HQL.
*/
// // DATAJPA-1233
// @Query(value = "SELECT u FROM User u ORDER BY CASE WHEN (u.firstname >= :name) THEN 0 ELSE 1 END, u.firstname")
// Page<User> findAllOrderedBySpecialNameSingleParam(@Param("name") String name, Pageable page);
//
// // DATAJPA-1233
// @Query(
// value = "SELECT u FROM User u WHERE :other = 'x' ORDER BY CASE WHEN (u.firstname >= :name) THEN 0 ELSE 1 END,
// u.firstname")
// Page<User> findAllOrderedBySpecialNameMultipleParams(@Param("name") String name, @Param("other") String other,
// Pageable page);
//
// // DATAJPA-1233
// @Query(
// value = "SELECT u FROM User u WHERE ?2 = 'x' ORDER BY CASE WHEN (u.firstname >= ?1) THEN 0 ELSE 1 END,
// u.firstname")
// Page<User> findAllOrderedBySpecialNameMultipleParamsIndexed(String other, String name, Pageable page);
@Query(value = "SELECT u FROM User u ORDER BY CASE WHEN (u.firstname >= :name) THEN 0 ELSE 1 END, u.firstname")
Page<User> findAllOrderedByNamedParam(@Param("name") String name, Pageable page);
@Query(value = "SELECT u FROM User u ORDER BY CASE WHEN (u.firstname >= ?1) THEN 0 ELSE 1 END, u.firstname")
Page<User> findAllOrderedByIndexedParam(String name, Pageable page);
@Query(
value = "SELECT u FROM User u WHERE :other = 'x' ORDER BY CASE WHEN (u.firstname >= :name) THEN 0 ELSE 1 END, u.firstname")
Page<User> findAllOrderedBySpecialNameMultipleParams(@Param("name") String name, @Param("other") String other,
Pageable page);
// Note that parameters used in the order-by statement are just cut off, so we must declare a query that parameter
// label order remains valid even after truncating the order by part. (i.e. WHERE ?2 = 'x' ORDER BY CASE WHEN
// (u.firstname >= ?1) isn't going to work).
@Query(
value = "SELECT u FROM User u WHERE ?1 = 'x' ORDER BY CASE WHEN (u.firstname >= ?2) THEN 0 ELSE 1 END, u.firstname")
Page<User> findAllOrderedBySpecialNameMultipleParamsIndexed(String other, String name, Pageable page);
// DATAJPA-928
Page<User> findByNativeNamedQueryWithPageable(Pageable pageable);