Skip to content

Commit 036342f

Browse files
committed
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
1 parent b4cb5b5 commit 036342f

File tree

4 files changed

+61
-7
lines changed

4 files changed

+61
-7
lines changed

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlCountQueryTransformer.java

+4
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectio
100100

101101
if (countSelection.requiresPrimaryAlias()) {
102102
// constructor
103+
if (primaryFromAlias == null) {
104+
throw new IllegalStateException(
105+
"Primary alias must be set for DISTINCT count selection using constructor expressions");
106+
}
103107
nested.append(QueryTokens.token(primaryFromAlias));
104108
} else {
105109
// keep all the select items to distinct against

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlCountQueryTransformer.java

+4
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectio
100100

101101
if (countSelection.requiresPrimaryAlias()) {
102102
// constructor
103+
if (primaryFromAlias == null) {
104+
throw new IllegalStateException(
105+
"Primary alias must be set for DISTINCT count selection using constructor expressions");
106+
}
103107
nested.append(QueryTokens.token(primaryFromAlias));
104108
} else {
105109
// keep all the select items to distinct against

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java

+36
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,42 @@ static CountSelectionTokenStream create(QueryTokenStream selection) {
7171
return new CountSelectionTokenStream(target, containsNew);
7272
}
7373

74+
/**
75+
* Filter constructor expression and return the selection list of the constructor.
76+
*
77+
* @return the selection list of the constructor without {@code NEW}, class name, and the first level of
78+
* parentheses.
79+
* @since 3.5.2
80+
*/
81+
public CountSelectionTokenStream withoutConstructorExpression() {
82+
83+
if (!requiresPrimaryAlias()) {
84+
return this;
85+
}
86+
87+
List<QueryToken> target = new ArrayList<>(size());
88+
int nestingLevel = 0;
89+
90+
for (QueryToken token : this) {
91+
92+
if (token.equals(TOKEN_OPEN_PAREN)) {
93+
nestingLevel++;
94+
continue;
95+
}
96+
97+
if (token.equals(TOKEN_CLOSE_PAREN)) {
98+
nestingLevel--;
99+
continue;
100+
}
101+
102+
if (nestingLevel > 0) {
103+
target.add(token);
104+
}
105+
}
106+
107+
return new CountSelectionTokenStream(target, requiresPrimaryAlias());
108+
}
109+
74110
@Override
75111
public Iterator<QueryToken> iterator() {
76112
return tokens.iterator();

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java

+17-7
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
*
4242
* @author Greg Turnquist
4343
* @author Christoph Strobl
44+
* @author Mark Paluch
4445
*/
4546
class HqlQueryTransformerTests {
4647

@@ -182,6 +183,12 @@ void multipleAliasesShouldBeGathered() {
182183

183184
@Test
184185
void createsCountQueryCorrectly() {
186+
187+
assertCountQuery("SELECT id FROM Person", "SELECT count(id) FROM Person");
188+
assertCountQuery("SELECT p.id FROM Person p", "SELECT count(p) FROM Person p");
189+
assertCountQuery("SELECT id FROM Person p", "SELECT count(id) FROM Person p");
190+
assertCountQuery("SELECT id, name FROM Person", "SELECT count(*) FROM Person");
191+
assertCountQuery("SELECT id, name FROM Person p", "SELECT count(p) FROM Person p");
185192
assertCountQuery(QUERY, COUNT_QUERY);
186193
}
187194

@@ -204,6 +211,9 @@ void createsCountQueryForConstructorQueries() {
204211

205212
assertCountQuery("select distinct new com.example.User(u.name) from User u where u.foo = ?1",
206213
"select count(distinct u) from User u where u.foo = ?1");
214+
215+
assertCountQuery("select distinct new com.example.User(name, lastname) from User where foo = ?1",
216+
"select count(distinct name, lastname) from User where foo = ?1");
207217
}
208218

209219
@Test
@@ -913,7 +923,7 @@ void queryParserPicksCorrectAliasAmidstMultipleAlises() {
913923
void countQueryShouldWorkEvenWithoutExplicitAlias() {
914924

915925
assertCountQuery("FROM BookError WHERE portal = :portal",
916-
"select count(__) FROM BookError AS __ WHERE portal = :portal");
926+
"select count(__) FROM BookError WHERE portal = :portal");
917927

918928
assertCountQuery("FROM BookError b WHERE portal = :portal",
919929
"select count(b) FROM BookError b WHERE portal = :portal");
@@ -1107,15 +1117,15 @@ void aliasesShouldNotOverlapWithSortProperties() {
11071117
@Test // GH-3269, GH-3689
11081118
void createsCountQueryUsingAliasCorrectly() {
11091119

1110-
assertCountQuery("select distinct 1 as x from Employee", "select count(distinct 1) from Employee AS __");
1111-
assertCountQuery("SELECT DISTINCT abc AS x FROM T", "SELECT count(DISTINCT abc) FROM T AS __");
1112-
assertCountQuery("select distinct a as x, b as y from Employee", "select count(distinct a, b) from Employee AS __");
1120+
assertCountQuery("select distinct 1 as x from Employee", "select count(distinct 1) from Employee");
1121+
assertCountQuery("SELECT DISTINCT abc AS x FROM T", "SELECT count(DISTINCT abc) FROM T");
1122+
assertCountQuery("select distinct a as x, b as y from Employee", "select count(distinct a, b) from Employee");
11131123
assertCountQuery("select distinct sum(amount) as x from Employee GROUP BY n",
1114-
"select count(distinct sum(amount)) from Employee AS __ GROUP BY n");
1124+
"select count(distinct sum(amount)) from Employee GROUP BY n");
11151125
assertCountQuery("select distinct a, b, sum(amount) as c, d from Employee GROUP BY n",
1116-
"select count(distinct a, b, sum(amount), d) from Employee AS __ GROUP BY n");
1126+
"select count(distinct a, b, sum(amount), d) from Employee GROUP BY n");
11171127
assertCountQuery("select distinct a, count(b) as c from Employee GROUP BY n",
1118-
"select count(distinct a, count(b)) from Employee AS __ GROUP BY n");
1128+
"select count(distinct a, count(b)) from Employee GROUP BY n");
11191129
assertCountQuery("select distinct substring(e.firstname, 1, position('a' in e.lastname)) as x from from Employee",
11201130
"select count(distinct substring(e.firstname, 1, position('a' in e.lastname))) from from Employee");
11211131
}

0 commit comments

Comments
 (0)