Skip to content

Commit 65f18f3

Browse files
mp911deschauder
mp911de
authored and
schauder
committed
Differentiate between JPQL and native queries in count query derivation.
We now consider whether a query is a native one when deriving a count query for pagination. Previously, the generated queries used JPQL syntax that doesn't comply with native SQL syntax rules. Closes #2773 Original pull request #2777
1 parent 4a81a40 commit 65f18f3

File tree

7 files changed

+216
-21
lines changed

7 files changed

+216
-21
lines changed

Diff for: ‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public String detectAlias() {
4646

4747
@Override
4848
public String createCountQueryFor(@Nullable String countProjection) {
49-
return QueryUtils.createCountQueryFor(this.query.getQueryString(), countProjection);
49+
return QueryUtils.createCountQueryFor(this.query.getQueryString(), countProjection, this.query.isNativeQuery());
5050
}
5151

5252
@Override

Diff for: ‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java

+17-6
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@
1515
*/
1616
package org.springframework.data.jpa.repository.query;
1717

18-
import static org.springframework.data.jpa.repository.query.JSqlParserUtils.getJSqlCount;
19-
import static org.springframework.data.jpa.repository.query.JSqlParserUtils.getJSqlLower;
20-
import static org.springframework.data.jpa.repository.query.QueryUtils.checkSortExpression;
18+
import static org.springframework.data.jpa.repository.query.JSqlParserUtils.*;
19+
import static org.springframework.data.jpa.repository.query.QueryUtils.*;
2120

2221
import net.sf.jsqlparser.JSQLParserException;
2322
import net.sf.jsqlparser.expression.Alias;
@@ -29,11 +28,23 @@
2928
import net.sf.jsqlparser.statement.delete.Delete;
3029
import net.sf.jsqlparser.statement.insert.Insert;
3130
import net.sf.jsqlparser.statement.merge.Merge;
32-
import net.sf.jsqlparser.statement.select.*;
31+
import net.sf.jsqlparser.statement.select.OrderByElement;
32+
import net.sf.jsqlparser.statement.select.PlainSelect;
33+
import net.sf.jsqlparser.statement.select.Select;
34+
import net.sf.jsqlparser.statement.select.SelectBody;
35+
import net.sf.jsqlparser.statement.select.SelectExpressionItem;
36+
import net.sf.jsqlparser.statement.select.SelectItem;
37+
import net.sf.jsqlparser.statement.select.SetOperationList;
38+
import net.sf.jsqlparser.statement.select.WithItem;
3339
import net.sf.jsqlparser.statement.update.Update;
3440
import net.sf.jsqlparser.statement.values.ValuesStatement;
3541

36-
import java.util.*;
42+
import java.util.ArrayList;
43+
import java.util.Collections;
44+
import java.util.HashSet;
45+
import java.util.List;
46+
import java.util.Objects;
47+
import java.util.Set;
3748
import java.util.stream.Collectors;
3849

3950
import org.springframework.data.domain.Sort;
@@ -400,7 +411,7 @@ public String createCountQueryFor(@Nullable String countProjection) {
400411
return selectBody.toString();
401412
}
402413

403-
String countProp = tableAlias == null ? "*" : tableAlias;
414+
String countProp = query.isNativeQuery() ? (distinct ? "*" : "1") : tableAlias == null ? "*" : tableAlias;
404415

405416
Function jSqlCount = getJSqlCount(Collections.singletonList(countProp), distinct);
406417
selectBody.setSelectItems(Collections.singletonList(new SelectExpressionItem(jSqlCount)));

Diff for: ‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

+37-6
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,23 @@
1818
import static jakarta.persistence.metamodel.Attribute.PersistentAttributeType.*;
1919
import static java.util.regex.Pattern.*;
2020

21-
import jakarta.persistence.*;
22-
import jakarta.persistence.criteria.*;
23-
import jakarta.persistence.metamodel.*;
21+
import jakarta.persistence.EntityManager;
22+
import jakarta.persistence.ManyToOne;
23+
import jakarta.persistence.OneToOne;
24+
import jakarta.persistence.Parameter;
25+
import jakarta.persistence.Query;
26+
import jakarta.persistence.criteria.CriteriaBuilder;
27+
import jakarta.persistence.criteria.Expression;
28+
import jakarta.persistence.criteria.Fetch;
29+
import jakarta.persistence.criteria.From;
30+
import jakarta.persistence.criteria.Join;
31+
import jakarta.persistence.criteria.JoinType;
32+
import jakarta.persistence.metamodel.Attribute;
2433
import jakarta.persistence.metamodel.Attribute.PersistentAttributeType;
34+
import jakarta.persistence.metamodel.Bindable;
35+
import jakarta.persistence.metamodel.ManagedType;
36+
import jakarta.persistence.metamodel.PluralAttribute;
37+
import jakarta.persistence.metamodel.SingularAttribute;
2538

2639
import java.lang.annotation.Annotation;
2740
import java.lang.reflect.AnnotatedElement;
@@ -570,6 +583,19 @@ public static String createCountQueryFor(String originalQuery) {
570583
*/
571584
@Deprecated
572585
public static String createCountQueryFor(String originalQuery, @Nullable String countProjection) {
586+
return createCountQueryFor(originalQuery, countProjection, false);
587+
}
588+
589+
/**
590+
* Creates a count projected query from the given original query.
591+
*
592+
* @param originalQuery must not be {@literal null}.
593+
* @param countProjection may be {@literal null}.
594+
* @param nativeQuery whether the underlying query is a native query.
595+
* @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}.
596+
* @since 2.7.8
597+
*/
598+
static String createCountQueryFor(String originalQuery, @Nullable String countProjection, boolean nativeQuery) {
573599

574600
Assert.hasText(originalQuery, "OriginalQuery must not be null or empty");
575601

@@ -591,9 +617,14 @@ public static String createCountQueryFor(String originalQuery, @Nullable String
591617

592618
String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue;
593619

594-
String alias = QueryUtils.detectAlias(originalQuery);
595-
if ("*".equals(variable) && alias != null) {
596-
replacement = alias;
620+
if (nativeQuery && (variable.contains(",") || "*".equals(variable))) {
621+
replacement = "1";
622+
} else {
623+
624+
String alias = QueryUtils.detectAlias(originalQuery);
625+
if (("*".equals(variable) && alias != null)) {
626+
replacement = alias;
627+
}
597628
}
598629

599630
countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));

Diff for: ‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancerUnitTests.java

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.repository.query;
17+
18+
/**
19+
* TCK Tests for {@link DefaultQueryEnhancer}.
20+
*
21+
* @author Mark Paluch
22+
*/
23+
public class DefaultQueryEnhancerUnitTests extends QueryEnhancerTckTests {
24+
25+
@Override
26+
QueryEnhancer createQueryEnhancer(DeclaredQuery declaredQuery) {
27+
return new DefaultQueryEnhancer(declaredQuery);
28+
}
29+
30+
}

Diff for: ‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.repository.query;
17+
18+
/**
19+
* TCK Tests for {@link JSqlParserQueryEnhancer}.
20+
*
21+
* @author Mark Paluch
22+
*/
23+
public class JSqlParserQueryEnhancerUnitTests extends QueryEnhancerTckTests {
24+
25+
@Override
26+
QueryEnhancer createQueryEnhancer(DeclaredQuery declaredQuery) {
27+
return new JSqlParserQueryEnhancer(declaredQuery);
28+
}
29+
30+
}

Diff for: ‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerTckTests.java

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.repository.query;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
20+
import java.util.stream.Stream;
21+
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.Arguments;
24+
import org.junit.jupiter.params.provider.MethodSource;
25+
26+
/**
27+
* TCK Tests for {@link QueryEnhancer}.
28+
*
29+
* @author Mark Paluch
30+
*/
31+
abstract class QueryEnhancerTckTests {
32+
33+
@ParameterizedTest
34+
@MethodSource("nativeCountQueries") // GH-2773
35+
void shouldDeriveNativeCountQuery(String query, String expected) {
36+
37+
DeclaredQuery declaredQuery = DeclaredQuery.of(query, true);
38+
QueryEnhancer enhancer = createQueryEnhancer(declaredQuery);
39+
String countQueryFor = enhancer.createCountQueryFor(null);
40+
41+
assertThat(countQueryFor).isEqualToIgnoringCase(expected);
42+
}
43+
44+
static Stream<Arguments> nativeCountQueries() {
45+
46+
return Stream.of(Arguments.of( //
47+
"SELECT * FROM table_name some_alias", //
48+
"select count(1) FROM table_name some_alias"), //
49+
50+
Arguments.of( //
51+
"SELECT name FROM table_name some_alias", //
52+
"select count(name) FROM table_name some_alias"), //
53+
54+
Arguments.of( //
55+
"SELECT DISTINCT name FROM table_name some_alias", //
56+
"select count(DISTINCT name) FROM table_name some_alias"));
57+
}
58+
59+
@ParameterizedTest // GH-2773
60+
@MethodSource("jpqlCountQueries")
61+
void shouldDeriveJpqlCountQuery(String query, String expected) {
62+
63+
DeclaredQuery declaredQuery = DeclaredQuery.of(query, false);
64+
QueryEnhancer enhancer = createQueryEnhancer(declaredQuery);
65+
String countQueryFor = enhancer.createCountQueryFor(null);
66+
67+
assertThat(countQueryFor).isEqualToIgnoringCase(expected);
68+
}
69+
70+
static Stream<Arguments> jpqlCountQueries() {
71+
72+
return Stream.of(Arguments.of( //
73+
"SELECT some_alias FROM table_name some_alias", //
74+
"select count(some_alias) FROM table_name some_alias"), //
75+
76+
Arguments.of( //
77+
"SELECT name FROM table_name some_alias", //
78+
"select count(name) FROM table_name some_alias"), //
79+
80+
Arguments.of( //
81+
"SELECT DISTINCT name FROM table_name some_alias", //
82+
"select count(DISTINCT name) FROM table_name some_alias"));
83+
}
84+
85+
abstract QueryEnhancer createQueryEnhancer(DeclaredQuery declaredQuery);
86+
87+
}

Diff for: ‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java

+14-8
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,12 @@ void createsCountQueryForNestedReferenceCorrectly() {
226226

227227
@Test // DATAJPA-420
228228
void createsCountQueryForScalarSelects() {
229-
assertCountQuery("select p.lastname,p.firstname from Person p", "select count(p) from Person p", true);
229+
assertCountQuery("select p.lastname,p.firstname from Person p", "select count(p) from Person p", false);
230+
}
231+
232+
@Test // DATAJPA-420
233+
void createsCountQueryForNativeScalarSelects() {
234+
assertCountQuery("select p.lastname,p.firstname from Person p", "select count(1) from Person p", true);
230235
}
231236

232237
@Test // DATAJPA-456
@@ -485,7 +490,7 @@ void createCountQuerySupportsWhitespaceCharacters() {
485490
" order by user.name\n ", true);
486491

487492
assertThat(getEnhancer(query).createCountQueryFor())
488-
.isEqualToIgnoringCase("select count(user) from User user where user.age = 18");
493+
.isEqualToIgnoringCase("select count(1) from User user where user.age = 18");
489494
}
490495

491496
@Test
@@ -498,7 +503,7 @@ void createCountQuerySupportsLineBreaksInSelectClause() {
498503
" order\nby\nuser.name\n ", true);
499504

500505
assertThat(getEnhancer(query).createCountQueryFor())
501-
.isEqualToIgnoringCase("select count(user) from User user where user.age = 18");
506+
.isEqualToIgnoringCase("select count(1) from User user where user.age = 18");
502507
}
503508

504509
@Test // DATAJPA-1061
@@ -719,17 +724,17 @@ void countQueryUsesCorrectVariable() {
719724

720725
QueryEnhancer queryEnhancer = getEnhancer(nativeQuery);
721726
String countQueryFor = queryEnhancer.createCountQueryFor();
722-
assertThat(countQueryFor).isEqualTo("SELECT count(*) FROM User WHERE created_at > $1");
727+
assertThat(countQueryFor).isEqualTo("SELECT count(1) FROM User WHERE created_at > $1");
723728

724729
nativeQuery = new StringQuery("SELECT * FROM (select * from test) ", true);
725730
queryEnhancer = getEnhancer(nativeQuery);
726731
countQueryFor = queryEnhancer.createCountQueryFor();
727-
assertThat(countQueryFor).isEqualTo("SELECT count(*) FROM (SELECT * FROM test)");
732+
assertThat(countQueryFor).isEqualTo("SELECT count(1) FROM (SELECT * FROM test)");
728733

729734
nativeQuery = new StringQuery("SELECT * FROM (select * from test) as test", true);
730735
queryEnhancer = getEnhancer(nativeQuery);
731736
countQueryFor = queryEnhancer.createCountQueryFor();
732-
assertThat(countQueryFor).isEqualTo("SELECT count(test) FROM (SELECT * FROM test) AS test");
737+
assertThat(countQueryFor).isEqualTo("SELECT count(1) FROM (SELECT * FROM test) AS test");
733738
}
734739

735740
@Test // GH-2555
@@ -859,7 +864,7 @@ void withStatementsWorksWithJSQLParser() {
859864

860865
assertThat(queryEnhancer.createCountQueryFor()).isEqualToIgnoringCase(
861866
"with sample_data (day, value) AS (VALUES ((0, 13), (1, 12), (2, 15), (3, 4), (4, 8), (5, 16)))\n"
862-
+ "SELECT count(a) FROM sample_data AS a");
867+
+ "SELECT count(1) FROM sample_data AS a");
863868
assertThat(queryEnhancer.applySorting(Sort.by("day").descending())).endsWith("ORDER BY a.day DESC");
864869
assertThat(queryEnhancer.getJoinAliases()).isEmpty();
865870
assertThat(queryEnhancer.detectAlias()).isEqualToIgnoringCase("a");
@@ -882,7 +887,7 @@ void multipleWithStatementsWorksWithJSQLParser() {
882887

883888
assertThat(queryEnhancer.createCountQueryFor()).isEqualToIgnoringCase(
884889
"with sample_data (day, value) AS (VALUES ((0, 13), (1, 12), (2, 15), (3, 4), (4, 8), (5, 16))),test2 AS (VALUES (1, 2, 3))\n"
885-
+ "SELECT count(a) FROM sample_data AS a");
890+
+ "SELECT count(1) FROM sample_data AS a");
886891
assertThat(queryEnhancer.applySorting(Sort.by("day").descending())).endsWith("ORDER BY a.day DESC");
887892
assertThat(queryEnhancer.getJoinAliases()).isEmpty();
888893
assertThat(queryEnhancer.detectAlias()).isEqualToIgnoringCase("a");
@@ -987,4 +992,5 @@ private static void endsIgnoringCase(String original, String endWithIgnoreCase)
987992
private static QueryEnhancer getEnhancer(DeclaredQuery query) {
988993
return QueryEnhancerFactory.forQuery(query);
989994
}
995+
990996
}

0 commit comments

Comments
 (0)