From 7e202f279aa28c48b08a70f9782bfe829ad0f26d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 1 Aug 2024 09:46:23 +0200 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-performance/pom.xml | 2 +- spring-data-jpa/pom.xml | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 0aea78d577..c2e2e8bd67 100755 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-3536-SNAPSHOT pom Spring Data JPA Parent diff --git a/spring-data-envers/pom.xml b/spring-data-envers/pom.xml index 8b836ae2f3..fcfaed2fe2 100755 --- a/spring-data-envers/pom.xml +++ b/spring-data-envers/pom.xml @@ -5,12 +5,12 @@ org.springframework.data spring-data-envers - 3.4.0-SNAPSHOT + 3.4.0-GH-3536-SNAPSHOT org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-3536-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-distribution/pom.xml b/spring-data-jpa-distribution/pom.xml index e9e1754bc3..6857d29280 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.4.0-SNAPSHOT + 3.4.0-GH-3536-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-performance/pom.xml b/spring-data-jpa-performance/pom.xml index 38f130983e..1f890d3e8d 100644 --- a/spring-data-jpa-performance/pom.xml +++ b/spring-data-jpa-performance/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-3536-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/pom.xml b/spring-data-jpa/pom.xml index 3006702796..c40e355a4d 100644 --- a/spring-data-jpa/pom.xml +++ b/spring-data-jpa/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jpa - 3.4.0-SNAPSHOT + 3.4.0-GH-3536-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-3536-SNAPSHOT ../pom.xml From 7f905e986282025755bbece7c750058e39d671aa Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 1 Aug 2024 09:50:01 +0200 Subject: [PATCH 2/3] Suppress selection item aliasing for the actual count query. We now no longer apply count selection filtering but rather skip select field aliasing when rendering a count query to drop the field alias within a count query. Previously, we removed field aliasing by filtering the token stream which also removed the AS keyword from cast operators. --- .../query/HqlCountQueryTransformer.java | 25 +++++++++++++------ .../query/HqlQueryTransformerTests.java | 25 +++++++++++++++---- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java index 537da47e71..4e083c757d 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java @@ -15,12 +15,7 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_AS; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_CLOSE_PAREN; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_COUNT_FUNC; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_DOUBLE_UNDERSCORE; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_OPEN_PAREN; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_SELECT_COUNT; +import static org.springframework.data.jpa.repository.query.QueryTokens.*; import org.springframework.data.jpa.repository.query.HqlParser.SelectClauseContext; import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder; @@ -208,6 +203,22 @@ public QueryTokenStream visitSelectClause(HqlParser.SelectClauseContext ctx) { return builder; } + @Override + public QueryTokenStream visitSelection(HqlParser.SelectionContext ctx) { + + if (isSubquery(ctx)) { + return super.visitSelection(ctx); + } + + QueryRendererBuilder builder = QueryRenderer.builder(); + + builder.append(visit(ctx.selectExpression())); + + // do not append variables to skip AS field aliasing + + return builder; + } + @Override public QueryRendererBuilder visitQueryOrder(HqlParser.QueryOrderContext ctx) { @@ -247,7 +258,7 @@ private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectio nested.append(QueryTokens.token(primaryFromAlias)); } else { // keep all the select items to distinct against - nested.append(countSelection); + nested.append(selectionListbuilder); } return nested; } 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 b4ce21db0a..759dfaf082 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 @@ -86,6 +86,23 @@ void applyCountToSimpleQuery() { assertThat(results).isEqualTo("select count(e) FROM Employee e where e.name = :name"); } + @Test // GH-3536 + void shouldCreateCountQueryForDistinctCount() { + + // given + var original = """ + select distinct cast(e.timestampField as date) as foo + from ExampleEntity e + order by cast(e.timestampField as date) desc + """; + + // when + var results = createCountQueryFor(original); + + // then + assertThat(results).isEqualTo("select count(distinct cast(e.timestampField as date)) from ExampleEntity e"); + } + @Test void applyCountToMoreComplexQuery() { @@ -701,11 +718,9 @@ void applySortingAccountsForNativeWindowFunction() { .isEqualTo("select dense_rank() over (order by lastname) from user u order by u.lastname, u.age desc"); // partition by + order by in over clause - assertThat( - createQueryFor( - "select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u", - sort)) - .isEqualTo( + assertThat(createQueryFor( + "select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u", + sort)).isEqualTo( "select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u order by u.age desc"); // partition by + order by in over clause + order by at the end From 6c94616f8f72f7cef2ac14bb0e84554f369ac9e0 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 1 Aug 2024 09:50:33 +0200 Subject: [PATCH 3/3] Polishing. Fix typos, move QueryTransformers.filterCountSelection to CountSelectionTokenStream.create. --- .../query/EqlCountQueryTransformer.java | 6 +- .../query/HqlCountQueryTransformer.java | 2 +- .../query/JpqlCountQueryTransformer.java | 2 +- .../repository/query/QueryTransformers.java | 58 +++++++++---------- .../query/EqlQueryTransformerTests.java | 2 +- .../query/JpqlQueryTransformerTests.java | 2 +- 6 files changed, 35 insertions(+), 37 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 6c328b1f36..aec9763fa0 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 @@ -15,9 +15,7 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_CLOSE_PAREN; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_COMMA; -import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_COUNT_FUNC; +import static org.springframework.data.jpa.repository.query.QueryTokens.*; import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder; import org.springframework.data.jpa.repository.query.QueryTransformers.CountSelectionTokenStream; @@ -98,7 +96,7 @@ public QueryTokenStream visitSelect_clause(EqlParser.Select_clauseContext ctx) { private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectionListbuilder) { QueryRendererBuilder nested = new QueryRendererBuilder(); - CountSelectionTokenStream countSelection = QueryTransformers.filterCountSelection(selectionListbuilder); + CountSelectionTokenStream countSelection = CountSelectionTokenStream.create(selectionListbuilder); if (countSelection.requiresPrimaryAlias()) { // constructor diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java index 4e083c757d..735bdae29c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlCountQueryTransformer.java @@ -251,7 +251,7 @@ private QueryRendererBuilder visitSubQuerySelectClause(SelectClauseContext ctx, private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectionListbuilder) { QueryRendererBuilder nested = new QueryRendererBuilder(); - CountSelectionTokenStream countSelection = QueryTransformers.filterCountSelection(selectionListbuilder); + CountSelectionTokenStream countSelection = CountSelectionTokenStream.create(selectionListbuilder); if (countSelection.requiresPrimaryAlias()) { // constructor 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 6ceb6e171a..2adac83e9b 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 @@ -96,7 +96,7 @@ public QueryRendererBuilder visitSelect_clause(JpqlParser.Select_clauseContext c private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectionListbuilder) { QueryRendererBuilder nested = new QueryRendererBuilder(); - CountSelectionTokenStream countSelection = QueryTransformers.filterCountSelection(selectionListbuilder); + CountSelectionTokenStream countSelection = CountSelectionTokenStream.create(selectionListbuilder); if (countSelection.requiresPrimaryAlias()) { // constructor 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 7481000ff8..46bdc36003 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 @@ -29,46 +29,46 @@ */ class QueryTransformers { - static CountSelectionTokenStream filterCountSelection(QueryTokenStream selection) { + static class CountSelectionTokenStream implements QueryTokenStream { - List target = new ArrayList<>(selection.size()); - boolean skipNext = false; - boolean containsNew = false; + private final List tokens; + private final boolean requiresPrimaryAlias; - for (QueryToken token : selection) { + CountSelectionTokenStream(List tokens, boolean requiresPrimaryAlias) { + this.tokens = tokens; + this.requiresPrimaryAlias = requiresPrimaryAlias; + } - if (skipNext) { - skipNext = false; - continue; - } + static CountSelectionTokenStream create(QueryTokenStream selection) { - if (token.equals(TOKEN_AS)) { - skipNext = true; - continue; - } + List target = new ArrayList<>(selection.size()); + boolean skipNext = false; + boolean containsNew = false; - if (!token.equals(TOKEN_COMMA) && token.isExpression()) { - token = QueryTokens.token(token.value()); - } + for (QueryToken token : selection) { - if (!containsNew && token.value().contains("new")) { - containsNew = true; - } + if (skipNext) { + skipNext = false; + continue; + } - target.add(token); - } + if (token.equals(TOKEN_AS)) { + skipNext = true; + continue; + } - return new CountSelectionTokenStream(target, containsNew); - } + if (!token.equals(TOKEN_COMMA) && token.isExpression()) { + token = QueryTokens.token(token.value()); + } - static class CountSelectionTokenStream implements QueryTokenStream { + if (!containsNew && token.value().contains("new")) { + containsNew = true; + } - private final List tokens; - private final boolean requiresPrimaryAlias; + target.add(token); + } - public CountSelectionTokenStream(List tokens, boolean requiresPrimaryAlias) { - this.tokens = tokens; - this.requiresPrimaryAlias = requiresPrimaryAlias; + return new CountSelectionTokenStream(target, containsNew); } @Override 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 52bdcd82d1..849c4abe5c 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 @@ -98,7 +98,7 @@ void applyCountToMoreComplexQuery() { } @Test - void applyCountToAlreadySorteQuery() { + void applyCountToAlreadySortedQuery() { // given var original = "SELECT e FROM Employee e where e.name = :name ORDER BY e.modified_date"; 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 eb04b8de07..59285245f2 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 @@ -99,7 +99,7 @@ void applyCountToMoreComplexQuery() { } @Test - void applyCountToAlreadySorteQuery() { + void applyCountToAlreadySortedQuery() { // given var original = "SELECT e FROM Employee e where e.name = :name ORDER BY e.modified_date";