From 2b668023d8017664a2d409ec91598d709ed831f3 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 11 Apr 2024 10:41:07 +0200 Subject: [PATCH] Polishing. Apply select list rewriting to EQL and JPQL parsers as well. See: #3269 Original pull request: #3276 --- .../repository/query/EqlQueryTransformer.java | 7 ++- .../repository/query/HqlQueryTransformer.java | 16 +---- .../query/JpaQueryParsingToken.java | 6 ++ .../query/JpqlQueryTransformer.java | 6 +- .../repository/query/QueryTransformers.java | 60 +++++++++++++++++++ .../query/EqlQueryTransformerTests.java | 14 +++++ .../query/JpqlQueryTransformerTests.java | 14 +++++ 7 files changed, 104 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 a3dcbd35e..890c5d39d 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 @@ class EqlQueryTransformer extends EqlQueryRenderer { 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 @@ class EqlQueryTransformer extends EqlQueryRenderer { 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 7183d1be3..f1e18f397 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 @@ class HqlQueryTransformer extends HqlQueryRenderer { 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 @@ class HqlQueryTransformer extends HqlQueryRenderer { 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 9ca84ce7f..0882174ef 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,12 @@ class JpaQueryParsingToken { return this.space; } + /** + * Compare whether the given {@link JpaQueryParsingToken token} is equal to the one held by this instance. + * + * @param token must not be {@literal null}. + * @return {@literal true} if both tokens are equals (using case-insensitive comparison). + */ boolean isA(JpaQueryParsingToken token) { return token.getToken().equalsIgnoreCase(this.getToken()); } 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 548824aa6..4c2f5f6c4 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 @@ class JpqlQueryTransformer extends JpqlQueryRenderer { 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 000000000..368e9a95b --- /dev/null +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java @@ -0,0 +1,60 @@ +/* + * 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 + * @since 3.2.5 + */ +class QueryTransformers { + + /** + * Filter a token list from a {@code SELECT} clause to be used within a count query. That is, filter any {@code AS …} + * aliases. + * + * @param selection the input selection. + * @return filtered selection to be used with count queries. + */ + 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 68c2da749..2b5e05252 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 @@ class EqlQueryTransformerTests { "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 ef4a04f04..72f0ff8b4 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 @@ class JpqlQueryTransformerTests { .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() {