From abdb652a2e61e47094a919c67b4daecdd286c1e8 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 18 Dec 2023 07:56:13 +0100 Subject: [PATCH 1/4] 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 dd336ca646..caf783258f 100755 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa-parent - 3.3.0-SNAPSHOT + 3.3.x-3269-SNAPSHOT pom Spring Data JPA Parent diff --git a/spring-data-envers/pom.xml b/spring-data-envers/pom.xml index 470678d048..7579f2cd52 100755 --- a/spring-data-envers/pom.xml +++ b/spring-data-envers/pom.xml @@ -5,12 +5,12 @@ org.springframework.data spring-data-envers - 3.3.0-SNAPSHOT + 3.3.x-3269-SNAPSHOT org.springframework.data spring-data-jpa-parent - 3.3.0-SNAPSHOT + 3.3.x-3269-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-distribution/pom.xml b/spring-data-jpa-distribution/pom.xml index 6bd074181c..9222700f21 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.3.0-SNAPSHOT + 3.3.x-3269-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/pom.xml b/spring-data-jpa/pom.xml index 67ad4cabb2..4d83aacf70 100644 --- a/spring-data-jpa/pom.xml +++ b/spring-data-jpa/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jpa - 3.3.0-SNAPSHOT + 3.3.x-3269-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.3.0-SNAPSHOT + 3.3.x-3269-SNAPSHOT ../pom.xml From b15171ca4344b45d69856abc496c8022a1849d5c Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 18 Dec 2023 13:04:46 +0100 Subject: [PATCH 2/4] Drop AS in count. --- .../jpa/repository/query/HqlQueryTransformer.java | 12 ++++++++++++ .../repository/query/HqlQueryTransformerTests.java | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java index 6e350def97..2a6482855a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; import org.antlr.v4.runtime.ParserRuleContext; import org.springframework.data.domain.Sort; @@ -357,6 +358,17 @@ public List visitSelectClause(HqlParser.SelectClauseContex if (ctx.DISTINCT() != null) { + List target = new ArrayList<>(selectionListTokens.size()); + for(int i=0;i hqlToken.getToken().contains("new"))) { // constructor tokens.add(new JpaQueryParsingToken(() -> primaryFromAlias)); 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 0b84deb614..c476f0d5bc 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 @@ -34,6 +34,7 @@ * Verify that HQL queries are properly transformed through the {@link JpaQueryEnhancer} and the {@link HqlQueryParser}. * * @author Greg Turnquist + * @author Christoph Strobl */ class HqlQueryTransformerTests { @@ -1031,6 +1032,17 @@ void aliasesShouldNotOverlapWithSortProperties() { "SELECT t3 FROM Test3 t3 JOIN t3.test2 x WHERE x.id = :test2Id order by t3.testDuplicateColumnName desc"); } + @Test // GH-3269 + 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 sum(amount) as x from Employee GROUP BY n","select count(distinct sum(amount)) from Employee AS __ 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"); + 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"); + } + private void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery); } From ee1061b04c2451b296d57c70a315775d38516682 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 18 Dec 2023 13:59:09 +0100 Subject: [PATCH 3/4] move to dedicated method --- .../repository/query/HqlQueryTransformer.java | 32 +++++++++++-------- .../query/JpaQueryParsingToken.java | 9 ++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java index 2a6482855a..7183d1be34 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; import org.antlr.v4.runtime.ParserRuleContext; import org.springframework.data.domain.Sort; @@ -31,6 +30,7 @@ * An ANTLR {@link org.antlr.v4.runtime.tree.ParseTreeVisitor} that transforms a parsed HQL query. * * @author Greg Turnquist + * @author Christoph Strobl * @since 3.1 */ class HqlQueryTransformer extends HqlQueryRenderer { @@ -358,23 +358,14 @@ public List visitSelectClause(HqlParser.SelectClauseContex if (ctx.DISTINCT() != null) { - List target = new ArrayList<>(selectionListTokens.size()); - for(int i=0;i countSelection = getCountSelection(selectionListTokens); - if (selectionListTokens.stream().anyMatch(hqlToken -> hqlToken.getToken().contains("new"))) { + if (countSelection.stream().anyMatch(hqlToken -> hqlToken.getToken().contains("new"))) { // constructor tokens.add(new JpaQueryParsingToken(() -> primaryFromAlias)); } else { // keep all the select items to distinct against - tokens.addAll(selectionListTokens); + tokens.addAll(countSelection); } } else { tokens.add(new JpaQueryParsingToken(() -> primaryFromAlias)); @@ -406,4 +397,19 @@ public List visitInstantiation(HqlParser.InstantiationCont static ArrayList newArrayList() { return new ArrayList<>(); } + + private static List getCountSelection(List selectionListTokens) { + + List target = new ArrayList<>(selectionListTokens.size()); + for (int i = 0; i < selectionListTokens.size(); i++) { + JpaQueryParsingToken token = selectionListTokens.get(i); + if (token.isA(TOKEN_AS)) { + i++; + continue; + } + target.add(token); + } + selectionListTokens = target; + return selectionListTokens; + } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryParsingToken.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryParsingToken.java index 7ae0c9fd0f..9ca84ce7ff 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryParsingToken.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryParsingToken.java @@ -131,6 +131,15 @@ boolean getSpace() { return this.space; } + boolean isA(JpaQueryParsingToken token) { + return token.getToken().equalsIgnoreCase(this.getToken()); + } + + @Override + public String toString() { + return getToken(); + } + /** * Switch the last {@link JpaQueryParsingToken}'s spacing to {@literal true}. */ From 4b0439a956276b6cc2d7eeeadd40455e029998ef Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 11 Apr 2024 10:41:07 +0200 Subject: [PATCH 4/4] Polishing. Apply select list rewriting to EQL and JPQL parsers as well. --- .../repository/query/EqlQueryTransformer.java | 7 ++- .../repository/query/HqlQueryTransformer.java | 16 +----- .../query/JpqlQueryTransformer.java | 6 ++- .../repository/query/QueryTransformers.java | 52 +++++++++++++++++++ .../query/EqlQueryTransformerTests.java | 14 +++++ .../query/JpqlQueryTransformerTests.java | 14 +++++ 6 files changed, 90 insertions(+), 19 deletions(-) create mode 100644 spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryTransformer.java index a3dcbd35ec..890c5d39d8 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryTransformer.java @@ -161,12 +161,14 @@ public List visitSelect_clause(EqlParser.Select_clauseCont if (ctx.DISTINCT() != null) { - if (selectItemTokens.stream().anyMatch(jpqlToken -> jpqlToken.getToken().contains("new"))) { + List countSelection = QueryTransformers.filterCountSelection(selectItemTokens); + + if (countSelection.stream().anyMatch(jpqlToken -> jpqlToken.getToken().contains("new"))) { // constructor tokens.add(new JpaQueryParsingToken(() -> primaryFromAlias)); } else { // keep all the select items to distinct against - tokens.addAll(selectItemTokens); + tokens.addAll(countSelection); } } else { tokens.add(new JpaQueryParsingToken(() -> primaryFromAlias)); @@ -240,4 +242,5 @@ public List visitConstructor_expression(EqlParser.Construc private static ArrayList newArrayList() { return new ArrayList<>(); } + } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java index 7183d1be34..f1e18f3970 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java @@ -358,7 +358,7 @@ public List visitSelectClause(HqlParser.SelectClauseContex if (ctx.DISTINCT() != null) { - List countSelection = getCountSelection(selectionListTokens); + List countSelection = QueryTransformers.filterCountSelection(selectionListTokens); if (countSelection.stream().anyMatch(hqlToken -> hqlToken.getToken().contains("new"))) { // constructor @@ -398,18 +398,4 @@ static ArrayList newArrayList() { return new ArrayList<>(); } - private static List getCountSelection(List selectionListTokens) { - - List target = new ArrayList<>(selectionListTokens.size()); - for (int i = 0; i < selectionListTokens.size(); i++) { - JpaQueryParsingToken token = selectionListTokens.get(i); - if (token.isA(TOKEN_AS)) { - i++; - continue; - } - target.add(token); - } - selectionListTokens = target; - return selectionListTokens; - } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java index 548824aa61..4c2f5f6c4e 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java @@ -161,12 +161,14 @@ public List visitSelect_clause(JpqlParser.Select_clauseCon if (ctx.DISTINCT() != null) { - if (selectItemTokens.stream().anyMatch(jpqlToken -> jpqlToken.getToken().contains("new"))) { + List countSelection = QueryTransformers.filterCountSelection(selectItemTokens); + + if (countSelection.stream().anyMatch(jpqlToken -> jpqlToken.getToken().contains("new"))) { // constructor tokens.add(new JpaQueryParsingToken(() -> primaryFromAlias)); } else { // keep all the select items to distinct against - tokens.addAll(selectItemTokens); + tokens.addAll(countSelection); } } else { tokens.add(new JpaQueryParsingToken(() -> primaryFromAlias)); 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 new file mode 100644 index 0000000000..7d37d9688a --- /dev/null +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java @@ -0,0 +1,52 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.repository.query; + +import static org.springframework.data.jpa.repository.query.JpaQueryParsingToken.*; + +import java.util.ArrayList; +import java.util.List; + +/** + * Utility class encapsulating common query transformations. + * + * @author Mark Paluch + */ +class QueryTransformers { + + static List filterCountSelection(List selection) { + + List target = new ArrayList<>(selection.size()); + boolean skipNext = false; + + for (JpaQueryParsingToken token : selection) { + + if (skipNext) { + skipNext = false; + continue; + } + + if (token.isA(TOKEN_AS)) { + skipNext = true; + continue; + } + target.add(token); + } + + return target; + } + +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java index 68c2da7499..2b5e052527 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java @@ -615,6 +615,20 @@ void countProjectionDistinctQueryIncludesNewLineAfterEntityAndBeforeWhere() { "SELECT count(DISTINCT entity1) FROM Entity1 entity1 LEFT JOIN entity1.entity2 entity2 ON entity1.key = entity2.key where entity1.id = 1799"); } + @Test // GH-3269 + void createsCountQueryUsingAliasCorrectly() { + + assertCountQuery("select distinct 1 as x from Employee e", "select count(distinct 1) from Employee e"); + assertCountQuery("SELECT DISTINCT abc AS x FROM T t", "SELECT count(DISTINCT abc) FROM T t"); + assertCountQuery("select distinct a as x, b as y from Employee e", "select count(distinct a , b) from Employee e"); + assertCountQuery("select distinct sum(amount) as x from Employee e GROUP BY n", + "select count(distinct sum(amount)) from Employee e GROUP BY n"); + assertCountQuery("select distinct a, b, sum(amount) as c, d from Employee e GROUP BY n", + "select count(distinct a, b, sum(amount) , d) from Employee e GROUP BY n"); + assertCountQuery("select distinct a, count(b) as c from Employee e GROUP BY n", + "select count(distinct a, count(b)) from Employee e GROUP BY n"); + } + @Test // GH-2393 void createCountQueryStartsWithWhitespace() { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java index ef4a04f04c..72f0ff8b49 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java @@ -679,6 +679,20 @@ void countQueryUsesCorrectVariable() { .isEqualTo("SELECT count(us) FROM users_statuses us WHERE (user_created_at BETWEEN :fromDate AND :toDate)"); } + @Test // GH-3269 + void createsCountQueryUsingAliasCorrectly() { + + assertCountQuery("select distinct 1 as x from Employee e", "select count(distinct 1) from Employee e"); + assertCountQuery("SELECT DISTINCT abc AS x FROM T t", "SELECT count(DISTINCT abc) FROM T t"); + assertCountQuery("select distinct a as x, b as y from Employee e", "select count(distinct a , b) from Employee e"); + assertCountQuery("select distinct sum(amount) as x from Employee e GROUP BY n", + "select count(distinct sum(amount)) from Employee e GROUP BY n"); + assertCountQuery("select distinct a, b, sum(amount) as c, d from Employee e GROUP BY n", + "select count(distinct a, b, sum(amount) , d) from Employee e GROUP BY n"); + assertCountQuery("select distinct a, count(b) as c from Employee e GROUP BY n", + "select count(distinct a, count(b)) from Employee e GROUP BY n"); + } + @Test // GH-2496, GH-2522, GH-2537, GH-2045 void orderByShouldWorkWithSubSelectStatements() {