diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index b31e630a4..8d7faa873 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -74,6 +74,8 @@ import org.springframework.util.StringUtils; * @author Andriy Redko * @author Peter Großmann * @author Greg Turnquist + * @author Diego Krupitza + * @author Jędrzej Biedrzycki */ public abstract class QueryUtils { @@ -107,7 +109,8 @@ public abstract class QueryUtils { private static final Pattern JOIN_PATTERN = Pattern.compile(JOIN, Pattern.CASE_INSENSITIVE); private static final String EQUALS_CONDITION_STRING = "%s.%s = :%s"; - private static final Pattern ORDER_BY = Pattern.compile(".*order\\s+by\\s+.*", CASE_INSENSITIVE); + private static final Pattern ORDER_BY = Pattern.compile("(order\\s+by\\s+)", CASE_INSENSITIVE); + private static final Pattern ORDER_BY_IN_WINDOW_OR_SUBSELECT = Pattern.compile("(\\(\\s*[a-z0-9 ,.*]*order\\s+by\\s+[a-z0-9 ,.]*\\s*\\))", CASE_INSENSITIVE); private static final Pattern NAMED_PARAMETER = Pattern.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER, CASE_INSENSITIVE); @@ -256,10 +259,10 @@ public abstract class QueryUtils { StringBuilder builder = new StringBuilder(query); - if (!ORDER_BY.matcher(query).matches()) { - builder.append(" order by "); - } else { + if (hasOrderByClause(query)) { builder.append(", "); + } else { + builder.append(" order by "); } Set joinAliases = getOuterJoinAliases(query); @@ -275,6 +278,31 @@ public abstract class QueryUtils { return builder.toString(); } + /** + * Returns {@code true} if the query has {@code order by} clause. + * The query has {@code order by} clause if there is an {@code order by} which is not part of window clause. + * + * @param query the analysed query string + * @return {@code true} if the query has {@code order by} clause, {@code false} otherwise + */ + private static boolean hasOrderByClause(String query) { + return countOccurences(ORDER_BY, query) > countOccurences(ORDER_BY_IN_WINDOW_OR_SUBSELECT, query); + } + + /** + * Counts the number of occurrences of the pattern in the string + * + * @param pattern regex with a group to match + * @param string analysed string + * @return the number of occurences of the pattern in the string + */ + private static int countOccurences(Pattern pattern, String string) { + Matcher matcher = pattern.matcher(string); + int occurences = 0; + while (matcher.find()) occurences++; + return occurences; + } + /** * Returns the order clause for the given {@link Order}. Will prefix the clause with the given alias if the referenced * property refers to a join alias, i.e. starts with {@code $alias.}. @@ -396,10 +424,12 @@ public abstract class QueryUtils { @Nullable @Deprecated public static String detectAlias(String query) { - + String alias = null; Matcher matcher = ALIAS_MATCH.matcher(query); - - return matcher.find() ? matcher.group(2) : null; + while (matcher.find()) { + alias = matcher.group(2); + } + return alias; } /** diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java index 25d03e7d7..8dcacfebf 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java @@ -40,6 +40,7 @@ import org.springframework.data.jpa.domain.JpaSort; * @author Grégoire Druant * @author Mohammad Hewedy * @author Greg Turnquist + * @author Jędrzej Biedrzycki */ class QueryUtilsUnitTests { @@ -554,4 +555,79 @@ class QueryUtilsUnitTests { private static void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery); } + + @Test // GH-2260 + void applySortingAccountsForNativeWindowFunction() { + + Sort sort = Sort.by(Order.desc("age")); + + // order by absent + assertThat(QueryUtils.applySorting("select * from user u", sort)) + .isEqualTo("select * from user u order by u.age desc"); + + // order by present + assertThat(QueryUtils.applySorting("select * from user u order by u.lastname", sort)) + .isEqualTo("select * from user u order by u.lastname, u.age desc"); + + // partition by + assertThat(QueryUtils.applySorting("select dense_rank() over (partition by age) from user u", sort)) + .isEqualTo("select dense_rank() over (partition by age) from user u order by u.age desc"); + + // order by in over clause + assertThat(QueryUtils.applySorting("select dense_rank() over (order by lastname) from user u", sort)) + .isEqualTo("select dense_rank() over (order by lastname) from user u order by u.age desc"); + + // order by in over clause (additional spaces) + assertThat(QueryUtils.applySorting("select dense_rank() over ( order by lastname ) from user u", sort)) + .isEqualTo("select dense_rank() over ( order by lastname ) from user u order by u.age desc"); + + // order by in over clause + at the end + assertThat( + QueryUtils.applySorting("select dense_rank() over (order by lastname) from user u order by u.lastname", sort)) + .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(QueryUtils.applySorting( + "select dense_rank() over (partition by active, age order by lastname) from user u", sort)).isEqualTo( + "select dense_rank() over (partition by active, age order by lastname) from user u order by u.age desc"); + + // partition by + order by in over clause + order by at the end + assertThat(QueryUtils.applySorting( + "select dense_rank() over (partition by active, age order by lastname) from user u order by active", sort)) + .isEqualTo( + "select dense_rank() over (partition by active, age order by lastname) from user u order by active, u.age desc"); + + // partition by + order by in over clause + frame clause + assertThat(QueryUtils.applySorting( + "select dense_rank() over ( partition by active, age order by username rows between current row and unbounded following ) from user u", + sort)).isEqualTo( + "select dense_rank() over ( partition by active, age order by username rows between current row and unbounded following ) from user u order by u.age desc"); + + // partition by + order by in over clause + frame clause + order by at the end + assertThat(QueryUtils.applySorting( + "select dense_rank() over ( partition by active, age order by username rows between current row and unbounded following ) from user u order by active", + sort)).isEqualTo( + "select dense_rank() over ( partition by active, age order by username rows between current row and unbounded following ) from user u order by active, u.age desc"); + + // order by in subselect (select expression) + assertThat( + QueryUtils.applySorting("select lastname, (select i.id from item i order by i.id limit 1) from user u", sort)) + .isEqualTo( + "select lastname, (select i.id from item i order by i.id limit 1) from user u order by u.age desc"); + + // order by in subselect (select expression) + at the end + assertThat(QueryUtils.applySorting( + "select lastname, (select i.id from item i order by 1 limit 1) from user u order by active", sort)).isEqualTo( + "select lastname, (select i.id from item i order by 1 limit 1) from user u order by active, u.age desc"); + + // order by in subselect (from expression) + assertThat(QueryUtils.applySorting("select * from (select * from user order by age desc limit 10) u", sort)) + .isEqualTo("select * from (select * from user order by age desc limit 10) u order by age desc"); + + // order by in subselect (from expression) + at the end + assertThat(QueryUtils.applySorting( + "select * from (select * from user order by 1, 2, 3 desc limit 10) u order by u.active asc", sort)).isEqualTo( + "select * from (select * from user order by 1, 2, 3 desc limit 10) u order by u.active asc, age desc"); + } + }