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 84a8b70e0e..215b724283 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 @@ -73,6 +73,7 @@ * @author Mohammad Hewedy * @author Andriy Redko * @author Peter Großmann + * @author Jędrzej Biedrzycki */ public abstract class QueryUtils { @@ -106,7 +107,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); @@ -255,10 +257,10 @@ public static String applySorting(String query, Sort sort, @Nullable String alia 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); @@ -274,6 +276,31 @@ public static String applySorting(String query, Sort sort, @Nullable String alia 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.}. @@ -389,10 +416,12 @@ private static String toJpaDirection(Order order) { @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 9040cfabf5..873677c622 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 @@ -39,6 +39,7 @@ * @author Florian Lüdiger * @author Grégoire Druant * @author Mohammad Hewedy + * @author Jędrzej Biedrzycki */ class QueryUtilsUnitTests { @@ -514,4 +515,53 @@ void findProjectionClauseWithIncludedFrom() { private static void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery); } + + @Test + 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"); + } + }