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 cd6ff2d62..c45c3d8aa 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 @@ -240,10 +240,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 0424d5ff6..8bc8dff0a 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 @@ -73,6 +73,15 @@ class StringQuery implements DeclaredQuery { * @param query must not be {@literal null} or empty. */ public StringQuery(String query, boolean isNative) { + this(query, isNative, it -> {}); + } + + /** + * Creates a new {@link StringQuery} from the given JPQL query. + * + * @param query must not be {@literal null} or empty. + */ + private StringQuery(String query, boolean isNative, Consumer> parameterPostProcessor) { Assert.hasText(query, "Query must not be null or empty"); @@ -87,6 +96,8 @@ class StringQuery implements DeclaredQuery { this.usesJdbcStyleParameters = queryMeta.usesJdbcStyleParameters; this.queryEnhancer = QueryEnhancerFactory.forQuery(this); + parameterPostProcessor.accept(this.bindings); + boolean hasNamedParameters = false; for (ParameterBinding parameterBinding : getParameterBindings()) { if (parameterBinding.getIdentifier().hasName() && parameterBinding.getOrigin().isMethodArgument()) { @@ -117,15 +128,26 @@ class StringQuery implements DeclaredQuery { @Override public DeclaredQuery deriveCountQuery(@Nullable String countQueryProjection) { - 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); - } + return new StringQuery(this.queryEnhancer.createCountQueryFor(countQueryProjection), // + this.isNative, derivedBindings -> { - return stringQuery; + // 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(derivedBindings)) { + + for (ParameterBinding binding : bindings) { + + if (binding.getOrigin().isExpression() && derivedBindings.removeIf( + it -> !it.getOrigin().isExpression() && it.getIdentifier().equals(binding.getIdentifier()))) { + derivedBindings.add(binding); + } + } + } + }); } @Override @@ -243,8 +265,7 @@ class StringQuery implements DeclaredQuery { } ValueExpressionQueryRewriter.ParsedQuery parsedQuery = createSpelExtractor(query, - parametersShouldBeAccessedByIndex, - greatestParameterIndex); + parametersShouldBeAccessedByIndex, greatestParameterIndex); String resultingQuery = parsedQuery.getQueryString(); Matcher matcher = PARAMETER_BINDING_PATTERN.matcher(resultingQuery); @@ -350,8 +371,7 @@ class StringQuery implements DeclaredQuery { } private static ValueExpressionQueryRewriter.ParsedQuery createSpelExtractor(String queryWithSpel, - boolean parametersShouldBeAccessedByIndex, - int greatestParameterIndex) { + 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 b04d6c7e0..78dc30791 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 @@ -3133,22 +3133,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 1df1abde1..2b8187182 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 @@ -177,7 +177,7 @@ class ExpressionBasedStringQueryUnitTests { } @Test - public void doesTemplatingWhenEntityNameSpelIsPresent() { + void doesTemplatingWhenEntityNameSpelIsPresent() { StringQuery query = new ExpressionBasedStringQuery("select #{#entityName + 'Hallo'} from #{#entityName} u", metadata, PARSER, false); @@ -186,7 +186,7 @@ class ExpressionBasedStringQueryUnitTests { } @Test - public void doesNoTemplatingWhenEntityNameSpelIsNotPresent() { + void doesNoTemplatingWhenEntityNameSpelIsNotPresent() { StringQuery query = new ExpressionBasedStringQuery("select #{#entityName + 'Hallo'} from User u", metadata, PARSER, false); @@ -195,7 +195,7 @@ class ExpressionBasedStringQueryUnitTests { } @Test - public void doesTemplatingWhenEntityNameSpelIsPresentForBindParameter() { + void doesTemplatingWhenEntityNameSpelIsPresentForBindParameter() { StringQuery query = new ExpressionBasedStringQuery("select u from #{#entityName} u where name = :#{#something}", metadata, 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 4db398991..04bf714ae 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 @@ -310,6 +310,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 68357a5c9..c4ebaf9f4 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 @@ -88,9 +88,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 @@ -424,7 +423,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 @@ -578,26 +578,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); @@ -751,7 +748,8 @@ public interface UserRepository extends JpaRepository, JpaSpecifi @Retention(RetentionPolicy.RUNTIME) @Query("select u, count(r) from User u left outer join u.roles r group by u") - @interface UserRoleCountProjectingQuery {} + @interface UserRoleCountProjectingQuery { + } interface RolesAndFirstname { @@ -781,10 +779,12 @@ public interface UserRepository extends JpaRepository, JpaSpecifi } - record UserRoleCountDtoProjection(User user, Long roleCount) {} + record UserRoleCountDtoProjection(User user, Long roleCount) { + } interface UserRoleCountInterfaceProjection { User getUser(); + Long getRoleCount(); }