From aa36f3c4dfeef2517387f737412a0ed56a3af2a2 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 14 Jan 2025 10:36:11 +0100 Subject: [PATCH] Polishing. Revisit DISTINCT count queries when primary alias isn't set. HQL can handle such queries, JPQL and EQL transformers now fail properly. See #3744 --- .../query/EqlCountQueryTransformer.java | 4 +++ .../query/JpqlCountQueryTransformer.java | 4 +++ .../repository/query/QueryTransformers.java | 36 +++++++++++++++++++ .../query/HqlQueryTransformerTests.java | 24 +++++++++---- 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlCountQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlCountQueryTransformer.java index 934683c824..0221aff83a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlCountQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlCountQueryTransformer.java @@ -99,6 +99,10 @@ private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectio if (countSelection.requiresPrimaryAlias()) { // constructor + if (primaryFromAlias == null) { + throw new IllegalStateException( + "Primary alias must be set for DISTINCT count selection using constructor expressions"); + } nested.append(QueryTokens.token(primaryFromAlias)); } else { // keep all the select items to distinct against diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlCountQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlCountQueryTransformer.java index b8fe1eedce..89e4d54070 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlCountQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlCountQueryTransformer.java @@ -100,6 +100,10 @@ private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectio if (countSelection.requiresPrimaryAlias()) { // constructor + if (primaryFromAlias == null) { + throw new IllegalStateException( + "Primary alias must be set for DISTINCT count selection using constructor expressions"); + } nested.append(QueryTokens.token(primaryFromAlias)); } else { // keep all the select items to distinct against diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java index e976550460..1a0372d9f5 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java @@ -71,6 +71,42 @@ static CountSelectionTokenStream create(QueryTokenStream selection) { return new CountSelectionTokenStream(target, containsNew); } + /** + * Filter constructor expression and return the selection list of the constructor. + * + * @return the selection list of the constructor without {@code NEW}, class name, and the first level of + * parentheses. + * @since 3.5.2 + */ + public CountSelectionTokenStream withoutConstructorExpression() { + + if (!requiresPrimaryAlias()) { + return this; + } + + List target = new ArrayList<>(size()); + int nestingLevel = 0; + + for (QueryToken token : this) { + + if (token.equals(TOKEN_OPEN_PAREN)) { + nestingLevel++; + continue; + } + + if (token.equals(TOKEN_CLOSE_PAREN)) { + nestingLevel--; + continue; + } + + if (nestingLevel > 0) { + target.add(token); + } + } + + return new CountSelectionTokenStream(target, requiresPrimaryAlias()); + } + @Override public Iterator iterator() { return tokens.iterator(); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java index 979fb0fe5d..867e3f87b2 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java @@ -41,6 +41,7 @@ * * @author Greg Turnquist * @author Christoph Strobl + * @author Mark Paluch */ class HqlQueryTransformerTests { @@ -182,6 +183,12 @@ void multipleAliasesShouldBeGathered() { @Test void createsCountQueryCorrectly() { + + assertCountQuery("SELECT id FROM Person", "SELECT count(id) FROM Person"); + assertCountQuery("SELECT p.id FROM Person p", "SELECT count(p) FROM Person p"); + assertCountQuery("SELECT id FROM Person p", "SELECT count(id) FROM Person p"); + assertCountQuery("SELECT id, name FROM Person", "SELECT count(*) FROM Person"); + assertCountQuery("SELECT id, name FROM Person p", "SELECT count(p) FROM Person p"); assertCountQuery(QUERY, COUNT_QUERY); } @@ -204,6 +211,9 @@ void createsCountQueryForConstructorQueries() { assertCountQuery("select distinct new com.example.User(u.name) from User u where u.foo = ?1", "select count(distinct u) from User u where u.foo = ?1"); + + assertCountQuery("select distinct new com.example.User(name, lastname) from User where foo = ?1", + "select count(distinct name, lastname) from User where foo = ?1"); } @Test @@ -913,7 +923,7 @@ void queryParserPicksCorrectAliasAmidstMultipleAlises() { void countQueryShouldWorkEvenWithoutExplicitAlias() { assertCountQuery("FROM BookError WHERE portal = :portal", - "select count(__) FROM BookError AS __ WHERE portal = :portal"); + "select count(__) FROM BookError WHERE portal = :portal"); assertCountQuery("FROM BookError b WHERE portal = :portal", "select count(b) FROM BookError b WHERE portal = :portal"); @@ -1107,15 +1117,15 @@ void aliasesShouldNotOverlapWithSortProperties() { @Test // GH-3269, GH-3689 void createsCountQueryUsingAliasCorrectly() { - assertCountQuery("select distinct 1 as x from Employee", "select count(distinct 1) from Employee AS __"); - assertCountQuery("SELECT DISTINCT abc AS x FROM T", "SELECT count(DISTINCT abc) FROM T AS __"); - assertCountQuery("select distinct a as x, b as y from Employee", "select count(distinct a, b) from Employee AS __"); + assertCountQuery("select distinct 1 as x from Employee", "select count(distinct 1) from Employee"); + assertCountQuery("SELECT DISTINCT abc AS x FROM T", "SELECT count(DISTINCT abc) FROM T"); + assertCountQuery("select distinct a as x, b as y from Employee", "select count(distinct a, b) from Employee"); assertCountQuery("select distinct sum(amount) as x from Employee GROUP BY n", - "select count(distinct sum(amount)) from Employee AS __ GROUP BY n"); + "select count(distinct sum(amount)) from Employee GROUP BY n"); assertCountQuery("select distinct a, b, sum(amount) as c, d from Employee GROUP BY n", - "select count(distinct a, b, sum(amount), d) from Employee AS __ GROUP BY n"); + "select count(distinct a, b, sum(amount), d) from Employee GROUP BY n"); assertCountQuery("select distinct a, count(b) as c from Employee GROUP BY n", - "select count(distinct a, count(b)) from Employee AS __ GROUP BY n"); + "select count(distinct a, count(b)) from Employee GROUP BY n"); assertCountQuery("select distinct substring(e.firstname, 1, position('a' in e.lastname)) as x from from Employee", "select count(distinct substring(e.firstname, 1, position('a' in e.lastname))) from from Employee"); }