Skip to content

Commit

Permalink
Differentiate between JPQL and native queries in count query derivation.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mp911de committed Feb 15, 2023
1 parent 9db5541 commit a851022
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public String detectAlias() {

@Override
public String createCountQueryFor(@Nullable String countProjection) {
return QueryUtils.createCountQueryFor(this.query.getQueryString(), countProjection);
return QueryUtils.createCountQueryFor(this.query.getQueryString(), countProjection, this.query.isNativeQuery());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
*/
package org.springframework.data.jpa.repository.query;

import static org.springframework.data.jpa.repository.query.JSqlParserUtils.getJSqlCount;
import static org.springframework.data.jpa.repository.query.JSqlParserUtils.getJSqlLower;
import static org.springframework.data.jpa.repository.query.QueryUtils.checkSortExpression;
import static org.springframework.data.jpa.repository.query.JSqlParserUtils.*;
import static org.springframework.data.jpa.repository.query.QueryUtils.*;

import net.sf.jsqlparser.JSQLParserException;
import net.sf.jsqlparser.expression.Alias;
Expand Down Expand Up @@ -414,7 +413,7 @@ public String createCountQueryFor(@Nullable String countProjection) {
return selectBody.toString();
}

String countProp = tableAlias == null ? "*" : tableAlias;
String countProp = query.isNativeQuery() ? (distinct ? "*" : "1") : tableAlias == null ? "*" : tableAlias;

Function jSqlCount = getJSqlCount(Collections.singletonList(countProp), distinct);
selectBody.setSelectItems(Collections.singletonList(new SelectExpressionItem(jSqlCount)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,19 @@ public static String createCountQueryFor(String originalQuery) {
*/
@Deprecated
public static String createCountQueryFor(String originalQuery, @Nullable String countProjection) {
return createCountQueryFor(originalQuery, countProjection, false);
}

/**
* Creates a count projected query from the given original query.
*
* @param originalQuery must not be {@literal null}.
* @param countProjection may be {@literal null}.
* @param nativeQuery whether the underlying query is a native query.
* @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}.
* @since 2.7.8
*/
static String createCountQueryFor(String originalQuery, @Nullable String countProjection, boolean nativeQuery) {

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

Expand All @@ -591,9 +604,14 @@ public static String createCountQueryFor(String originalQuery, @Nullable String

String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue;

String alias = QueryUtils.detectAlias(originalQuery);
if ("*".equals(variable) && alias != null) {
replacement = alias;
if (nativeQuery && (variable.contains(",") || "*".equals(variable))) {
replacement = "1";
} else {

String alias = QueryUtils.detectAlias(originalQuery);
if (("*".equals(variable) && alias != null)) {
replacement = alias;
}
}

countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,12 @@ void createsCountQueryForNestedReferenceCorrectly() {

@Test // DATAJPA-420
void createsCountQueryForScalarSelects() {
assertCountQuery("select p.lastname,p.firstname from Person p", "select count(p) from Person p", true);
assertCountQuery("select p.lastname,p.firstname from Person p", "select count(p) from Person p", false);
}

@Test // DATAJPA-420
void createsCountQueryForNativeScalarSelects() {
assertCountQuery("select p.lastname,p.firstname from Person p", "select count(1) from Person p", true);
}

@Test // DATAJPA-456
Expand Down Expand Up @@ -487,7 +492,7 @@ void createCountQuerySupportsWhitespaceCharacters() {
" order by user.name\n ", true);

assertThat(getEnhancer(query).createCountQueryFor())
.isEqualToIgnoringCase("select count(user) from User user where user.age = 18");
.isEqualToIgnoringCase("select count(1) from User user where user.age = 18");
}

@Test
Expand All @@ -500,7 +505,7 @@ void createCountQuerySupportsLineBreaksInSelectClause() {
" order\nby\nuser.name\n ", true);

assertThat(getEnhancer(query).createCountQueryFor())
.isEqualToIgnoringCase("select count(user) from User user where user.age = 18");
.isEqualToIgnoringCase("select count(1) from User user where user.age = 18");
}

@Test // DATAJPA-1061
Expand Down Expand Up @@ -721,17 +726,17 @@ void countQueryUsesCorrectVariable() {

QueryEnhancer queryEnhancer = getEnhancer(nativeQuery);
String countQueryFor = queryEnhancer.createCountQueryFor();
assertThat(countQueryFor).isEqualTo("SELECT count(*) FROM User WHERE created_at > $1");
assertThat(countQueryFor).isEqualTo("SELECT count(1) FROM User WHERE created_at > $1");

nativeQuery = new StringQuery("SELECT * FROM (select * from test) ", true);
queryEnhancer = getEnhancer(nativeQuery);
countQueryFor = queryEnhancer.createCountQueryFor();
assertThat(countQueryFor).isEqualTo("SELECT count(*) FROM (SELECT * FROM test)");
assertThat(countQueryFor).isEqualTo("SELECT count(1) FROM (SELECT * FROM test)");

nativeQuery = new StringQuery("SELECT * FROM (select * from test) as test", true);
queryEnhancer = getEnhancer(nativeQuery);
countQueryFor = queryEnhancer.createCountQueryFor();
assertThat(countQueryFor).isEqualTo("SELECT count(test) FROM (SELECT * FROM test) AS test");
assertThat(countQueryFor).isEqualTo("SELECT count(1) FROM (SELECT * FROM test) AS test");
}

@Test // GH-2555
Expand Down Expand Up @@ -861,7 +866,7 @@ void withStatementsWorksWithJSQLParser() {

assertThat(queryEnhancer.createCountQueryFor()).isEqualToIgnoringCase(
"with sample_data (day, value) AS (VALUES ((0, 13), (1, 12), (2, 15), (3, 4), (4, 8), (5, 16)))\n"
+ "SELECT count(a) FROM sample_data AS a");
+ "SELECT count(1) FROM sample_data AS a");
assertThat(queryEnhancer.applySorting(Sort.by("day").descending())).endsWith("ORDER BY a.day DESC");
assertThat(queryEnhancer.getJoinAliases()).isEmpty();
assertThat(queryEnhancer.detectAlias()).isEqualToIgnoringCase("a");
Expand All @@ -884,7 +889,7 @@ void multipleWithStatementsWorksWithJSQLParser() {

assertThat(queryEnhancer.createCountQueryFor()).isEqualToIgnoringCase(
"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"
+ "SELECT count(a) FROM sample_data AS a");
+ "SELECT count(1) FROM sample_data AS a");
assertThat(queryEnhancer.applySorting(Sort.by("day").descending())).endsWith("ORDER BY a.day DESC");
assertThat(queryEnhancer.getJoinAliases()).isEmpty();
assertThat(queryEnhancer.detectAlias()).isEqualToIgnoringCase("a");
Expand Down Expand Up @@ -989,4 +994,5 @@ private static void endsIgnoringCase(String original, String endWithIgnoreCase)
private static QueryEnhancer getEnhancer(DeclaredQuery query) {
return QueryEnhancerFactory.forQuery(query);
}

}

0 comments on commit a851022

Please sign in to comment.