From e6f13e65d4efe73f083d6ad2d3998368a404a96b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 23 Feb 2023 15:53:55 +0100 Subject: [PATCH 1/3] Prepare issue branch. --- pom.xml | 2 +- spring-data-envers/pom.xml | 4 ++-- spring-data-jpa-distribution/pom.xml | 2 +- spring-data-jpa/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index c94ae35cd5..bb51af4768 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa-parent - 3.1.0-SNAPSHOT + 3.1.0-GH-2812-SNAPSHOT pom Spring Data JPA Parent diff --git a/spring-data-envers/pom.xml b/spring-data-envers/pom.xml index db915d7c3b..740f4ee2c1 100755 --- a/spring-data-envers/pom.xml +++ b/spring-data-envers/pom.xml @@ -5,12 +5,12 @@ org.springframework.data spring-data-envers - 3.1.0-SNAPSHOT + 3.1.0-GH-2812-SNAPSHOT org.springframework.data spring-data-jpa-parent - 3.1.0-SNAPSHOT + 3.1.0-GH-2812-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-distribution/pom.xml b/spring-data-jpa-distribution/pom.xml index a5cb2f09b5..4d33fc3d63 100644 --- a/spring-data-jpa-distribution/pom.xml +++ b/spring-data-jpa-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-jpa-parent - 3.1.0-SNAPSHOT + 3.1.0-GH-2812-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/pom.xml b/spring-data-jpa/pom.xml index 95a83aac44..4f488514ad 100644 --- a/spring-data-jpa/pom.xml +++ b/spring-data-jpa/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jpa - 3.1.0-SNAPSHOT + 3.1.0-GH-2812-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.1.0-SNAPSHOT + 3.1.0-GH-2812-SNAPSHOT ../pom.xml From 3c572592ba6b21581a54522b9460af8e3b867253 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 23 Feb 2023 15:55:31 +0100 Subject: [PATCH 2/3] Delay count query derivation. We now delay the count query creation to the actual time when we need the count query to avoid query creation of invalid queries (e.g. count queries for DELETE or UPDATE statements). See #2812 --- .../query/AbstractStringBasedJpaQuery.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index e3a2ff1ce8..cf22cc9f40 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -18,15 +18,13 @@ import jakarta.persistence.EntityManager; import jakarta.persistence.Query; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.QueryRewriter; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; +import org.springframework.data.util.Lazy; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -46,7 +44,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { private final DeclaredQuery query; - private final DeclaredQuery countQuery; + private final Lazy countQuery; private final QueryMethodEvaluationContextProvider evaluationContextProvider; private final SpelExpressionParser parser; private final QueryParameterSetter.QueryMetadataCache metadataCache = new QueryParameterSetter.QueryMetadataCache(); @@ -65,8 +63,8 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { * @param queryRewriter must not be {@literal null}. */ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, String queryString, - @Nullable String countQueryString, QueryRewriter queryRewriter, QueryMethodEvaluationContextProvider evaluationContextProvider, - SpelExpressionParser parser) { + @Nullable String countQueryString, QueryRewriter queryRewriter, + QueryMethodEvaluationContextProvider evaluationContextProvider, SpelExpressionParser parser) { super(method, em); @@ -79,9 +77,10 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri this.query = new ExpressionBasedStringQuery(queryString, method.getEntityInformation(), parser, method.isNativeQuery()); - DeclaredQuery countQuery = query.deriveCountQuery(countQueryString, method.getCountQueryProjection()); - this.countQuery = ExpressionBasedStringQuery.from(countQuery, method.getEntityInformation(), parser, - method.isNativeQuery()); + this.countQuery = Lazy.of(() -> { + DeclaredQuery countQuery = query.deriveCountQuery(countQueryString, method.getCountQueryProjection()); + return ExpressionBasedStringQuery.from(countQuery, method.getEntityInformation(), parser, method.isNativeQuery()); + }); this.parser = parser; this.queryRewriter = queryRewriter; @@ -117,7 +116,7 @@ protected ParameterBinder createBinder() { @Override protected Query doCreateCountQuery(JpaParametersParameterAccessor accessor) { - String queryString = countQuery.getQueryString(); + String queryString = countQuery.get().getQueryString(); EntityManager em = getEntityManager(); Query query = getQueryMethod().isNativeQuery() // @@ -142,7 +141,7 @@ public DeclaredQuery getQuery() { * @return the countQuery */ public DeclaredQuery getCountQuery() { - return countQuery; + return countQuery.get(); } /** From ece6b213beff686413d668ac2f6ff5e16b1e3456 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 23 Feb 2023 15:56:02 +0100 Subject: [PATCH 3/3] Fix NullPointer when deriving a count query for non-SELECT statements. Closes #2812 --- .../data/jpa/repository/query/DeclaredQuery.java | 2 +- .../data/jpa/repository/query/QueryUtils.java | 2 +- .../jpa/repository/query/DefaultQueryUtilsUnitTests.java | 9 +++++++++ .../jpa/repository/query/QueryEnhancerUnitTests.java | 9 +++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java index 500e64d1ac..4821865def 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java @@ -105,7 +105,7 @@ default boolean usesPaging() { /** * Return whether the query is a native query of not. - * + * * @return true if native query otherwise false */ default boolean isNativeQuery() { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index 785dde580d..f581f191ff 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -617,7 +617,7 @@ static String createCountQueryFor(String originalQuery, @Nullable String countPr String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue; - if (nativeQuery && (variable.contains(",") || "*".equals(variable))) { + if (variable != null && (nativeQuery && (variable.contains(",") || "*".equals(variable)))) { replacement = "1"; } else { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryUtilsUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryUtilsUnitTests.java index 89dba5b884..2409a9b2d2 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryUtilsUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryUtilsUnitTests.java @@ -70,6 +70,15 @@ void createsCountQueryForDistinctQueries() { "select count(distinct u) from User u where u.foo = ?"); } + @Test // GH-2812 + void createsCountQueryForDeleteQuery() { + + String result = createCountQueryFor("delete from some_table where id in :ids", null, true); + + // ح(•̀ж•́)ง † + assertThat(result).isEqualTo("deleteselect count(where) from some_table where id in :ids"); + } + @Test void createsCountQueryForConstructorQueries() { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java index 549528160f..cdc9919fab 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java @@ -172,6 +172,15 @@ void findsExistingOrderByIndependentOfCase() { assertThat(query).endsWithIgnoringCase("ORDER BY p.firstname, p.lastname asc"); } + @Test // GH-2812 + void createCountQueryFromDeleteQuery() { + + StringQuery query = new StringQuery("delete from some_table where id in :ids", true); + + assertThat(getEnhancer(query).createCountQueryFor("p.lastname")) + .isEqualToIgnoringCase("delete from some_table where id in :ids"); + } + @Test // DATAJPA-456 void createCountQueryFromTheGivenCountProjection() {