diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java index 8bdeea1f0..5682d909c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetterFactory.java @@ -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 // diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index 4a2e9d8e2..1a26f381f 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -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 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 diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index 11df50869..98b133495 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -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 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 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 diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ExpressionBasedStringQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ExpressionBasedStringQueryUnitTests.java index 91b4ecea8..168189c6b 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ExpressionBasedStringQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ExpressionBasedStringQueryUnitTests.java @@ -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); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java index 5e19aaddb..a44267a7b 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java @@ -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() { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index eec409f62..4a88310b4 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -82,9 +82,8 @@ public interface UserRepository extends JpaRepository, JpaSpecifi java.util.Optional 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, 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 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 findByWithSpelParameterOnlyUsedForCountQuery(String lastname, Pageable page); // DATAJPA-564 @@ -563,26 +563,23 @@ public interface UserRepository extends JpaRepository, JpaSpecifi @Query("SELECT u FROM User u where u.firstname >= ?1 and u.lastname = '000:1'") List 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 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 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 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 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 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 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 findAllOrderedBySpecialNameMultipleParamsIndexed(String other, String name, Pageable page); // DATAJPA-928 Page findByNativeNamedQueryWithPageable(Pageable pageable);