Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DATAJPA-1435 - change COMPLEX_COUNT_VALUE for native query pageable sql. #381

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
* @author Jens Schauder
* @author Tom Hombergs
* @author David Madden
* @author Chao Jiang
*/
abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {

Expand Down Expand Up @@ -66,7 +67,7 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri
this.evaluationContextProvider = evaluationContextProvider;
this.query = new ExpressionBasedStringQuery(queryString, method.getEntityInformation(), parser);

DeclaredQuery countQuery = query.deriveCountQuery(method.getCountQuery(), method.getCountQueryProjection());
DeclaredQuery countQuery = query.deriveCountQuery(method.getCountQuery(), method.getCountQueryProjection(), method.isNativeQuery());
this.countQuery = ExpressionBasedStringQuery.from(countQuery, method.getEntityInformation(), parser);

this.parser = parser;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* A wrapper for a String representation of a query offering information about the query.
*
* @author Jens Schauder
* @author Chao Jiang
* @since 2.0.3
*/
interface DeclaredQuery {
Expand Down Expand Up @@ -80,9 +81,10 @@ static DeclaredQuery of(@Nullable String query) {
*
* @param countQuery an optional query string to be used if present.
* @param countQueryProjection an optional return type for the query.
* @param isNativeQuery true if query is native query.
* @return a new {@literal DeclaredQuery} instance.
*/
DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection);
DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection, Boolean isNativeQuery);

/**
* @return whether paging is implemented in the query itself, e.g. using SpEL expressions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* NULL-Object pattern implementation for {@link DeclaredQuery}.
*
* @author Jens Schauder
* @author Chao Jiang
* @since 2.0.3
*/
class EmptyDeclaredQuery implements DeclaredQuery {
Expand Down Expand Up @@ -93,7 +94,7 @@ public List<StringQuery.ParameterBinding> getParameterBindings() {
* @see org.springframework.data.jpa.repository.query.DeclaredQuery#deriveCountQuery(java.lang.String, java.lang.String)
*/
@Override
public DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection) {
public DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection, @Nullable Boolean isNativeQuery) {

Assert.hasText(countQuery, "CountQuery must not be empty!");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
* @author Oliver Gierke
* @author Thomas Darimont
* @author Mark Paluch
* @author Chao Jiang
*/
final class NamedQuery extends AbstractJpaQuery {

Expand Down Expand Up @@ -186,7 +187,7 @@ protected TypedQuery<Long> doCreateCountQuery(Object[] values) {

} else {

String countQueryString = declaredQuery.deriveCountQuery(null, countProjection).getQueryString();
String countQueryString = declaredQuery.deriveCountQuery(null, countProjection, false).getQueryString();

countQuery = em.createQuery(countQueryString, Long.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
* @author Nils Borrmann
* @author Reda.Housni-Alaoui
* @author Florian Lüdiger
* @author Chao Jiang
*/
public abstract class QueryUtils {

Expand All @@ -93,8 +94,10 @@ public abstract class QueryUtils {
static final String IDENTIFIER_GROUP = String.format("(%s)", IDENTIFIER);

private static final String COUNT_REPLACEMENT_TEMPLATE = "select count(%s) $5$6$7";
private static final String COUNT_REPLACEMENT_TEMPLATE_NATIVE = "select count(1) from (select %s $5$6$7) as total";
private static final String SIMPLE_COUNT_VALUE = "$2";
private static final String COMPLEX_COUNT_VALUE = "$3$6";
private static final String COMPLEX_COUNT_VALUE_NATIVE = "$3$4";
private static final String ORDER_BY_PART = "(?iu)\\s+order\\s+by\\s+.*$";

private static final Pattern ALIAS_MATCH;
Expand Down Expand Up @@ -426,20 +429,21 @@ public static <T> Query applyAndBind(String queryString, Iterable<T> entities, E
*/
@Deprecated
public static String createCountQueryFor(String originalQuery) {
return createCountQueryFor(originalQuery, null);
return createCountQueryFor(originalQuery, null, null);
}

/**
* Creates a count projected query from the given original query.
*
* @param originalQuery must not be {@literal null}.
* @param countProjection may be {@literal null}.
* @param isNativeQuery may be {@literal null}.
* @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}.
* @since 1.6
* @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead.
*/
@Deprecated
public static String createCountQueryFor(String originalQuery, @Nullable String countProjection) {
public static String createCountQueryFor(String originalQuery, @Nullable String countProjection, @Nullable Boolean isNativeQuery) {

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

Expand All @@ -452,8 +456,16 @@ public static String createCountQueryFor(String originalQuery, @Nullable String
boolean useVariable = variable != null && StringUtils.hasText(variable) && !variable.startsWith("new")
&& !variable.startsWith("count(") && !variable.contains(",");

String replacement = useVariable ? SIMPLE_COUNT_VALUE : COMPLEX_COUNT_VALUE;
countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));

if(isNativeQuery == null || !isNativeQuery) {

String replacement = useVariable ? SIMPLE_COUNT_VALUE : COMPLEX_COUNT_VALUE;
countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));
} else {

String replacement = useVariable ? SIMPLE_COUNT_VALUE : COMPLEX_COUNT_VALUE_NATIVE;
countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE_NATIVE, replacement));
}
} else {
countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, countProjection));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
* @author Oliver Wehrens
* @author Mark Paluch
* @author Jens Schauder
* @author Chao Jiang
*/
class StringQuery implements DeclaredQuery {

Expand Down Expand Up @@ -104,10 +105,10 @@ public List<ParameterBinding> getParameterBindings() {
*/
@Override
@SuppressWarnings("deprecation")
public DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection) {
public DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection, @Nullable Boolean isNativeQuery) {

return DeclaredQuery
.of(countQuery != null ? countQuery : QueryUtils.createCountQueryFor(query, countQueryProjection));
.of(countQuery != null ? countQuery : QueryUtils.createCountQueryFor(query, countQueryProjection, isNativeQuery));
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
* @author Christoph Strobl
* @author Jens Schauder
* @author Florian Lüdiger
* @author Chao Jiang
*/
public class QueryUtilsUnitTests {

Expand Down Expand Up @@ -228,9 +229,33 @@ public void createsCountQueryForScalarSelects() {

@Test // DATAJPA-456
public void createCountQueryFromTheGivenCountProjection() {
assertThat(createCountQueryFor("select p.lastname,p.firstname from Person p", "p.lastname"),
assertThat(createCountQueryFor("select p.lastname,p.firstname from Person p", "p.lastname", null),
is("select count(p.lastname) from Person p"));
}

@Test // DATAJPA-1435
public void createCountQueryFromTheGivenComplexCountProjection() {
String nativeQuery = "select p.lastname,p.firstname from Person_table p";
String jpqlQuery = "select p.lastname,p.firstname from Person p";
Boolean isNativeQuery = true;
Boolean isJpqlQuery = false;

//parse native sql, get correct query(use count(1) in select clause with subquery in from clause)
assertThat(createCountQueryFor(nativeQuery, null, isNativeQuery),
is("select count(1) from (select p.lastname,p.firstname from Person_table p) as total"));

//parse jpql, get correct query(use count(entity name) in select clause)
assertThat(createCountQueryFor(jpqlQuery, null, isJpqlQuery),
is("select count(p) from Person p"));

//parse native sql as jpql, count(entity name) not work
assertThat(createCountQueryFor(nativeQuery, null, isJpqlQuery),
is("select count(p) from Person_table p"));

//parse jpql as native sql, subquery not work
assertThat(createCountQueryFor(jpqlQuery, null, isNativeQuery),
is("select count(1) from (select p.lastname,p.firstname from Person p) as total"));
}

@Test // DATAJPA-726
public void detectsAliassesInPlainJoins() {
Expand Down