Skip to content

Commit b435b2c

Browse files
committed
Change "order by" pattern to match subqueries.
Fix alias detection bug occurring when using subqueries. Closes spring-projects#2260
1 parent a0f3b8c commit b435b2c

File tree

2 files changed

+29
-5
lines changed

2 files changed

+29
-5
lines changed

src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

+7-5
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public abstract class QueryUtils {
108108

109109
private static final String EQUALS_CONDITION_STRING = "%s.%s = :%s";
110110
private static final Pattern ORDER_BY = Pattern.compile("(order\\s+by\\s+)", CASE_INSENSITIVE);
111-
private static final Pattern ORDER_BY_IN_WINDOW_CLAUSE = Pattern.compile("(over\\s*\\(\\s*[a-z ,]*order\\s+by\\s+[a-z ,]*\\s*\\))", CASE_INSENSITIVE);
111+
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);
112112

113113
private static final Pattern NAMED_PARAMETER = Pattern.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER,
114114
CASE_INSENSITIVE);
@@ -284,7 +284,7 @@ public static String applySorting(String query, Sort sort, @Nullable String alia
284284
* @return {@code true} if the query has {@code order by} clause, {@code false} otherwise
285285
*/
286286
private static boolean hasOrderByClause(String query) {
287-
return countOccurences(ORDER_BY, query) > countOccurences(ORDER_BY_IN_WINDOW_CLAUSE, query);
287+
return countOccurences(ORDER_BY, query) > countOccurences(ORDER_BY_IN_WINDOW_OR_SUBSELECT, query);
288288
}
289289

290290
/**
@@ -416,10 +416,12 @@ private static String toJpaDirection(Order order) {
416416
@Nullable
417417
@Deprecated
418418
public static String detectAlias(String query) {
419-
419+
String alias = null;
420420
Matcher matcher = ALIAS_MATCH.matcher(query);
421-
422-
return matcher.find() ? matcher.group(2) : null;
421+
while (matcher.find()) {
422+
alias = matcher.group(2);
423+
}
424+
return alias;
423425
}
424426

425427
/**

src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java

+22
Original file line numberDiff line numberDiff line change
@@ -520,26 +520,48 @@ private static void assertCountQuery(String originalQuery, String countQuery) {
520520
void applySortingAccountsForNativeWindowFunction() {
521521
Sort sort = Sort.by(Order.desc("age"));
522522

523+
// order by absent
523524
assertThat(QueryUtils.applySorting("select * from user u", sort))
524525
.isEqualTo("select * from user u order by u.age desc");
526+
// order by present
525527
assertThat(QueryUtils.applySorting("select * from user u order by u.lastname", sort))
526528
.isEqualTo("select * from user u order by u.lastname, u.age desc");
529+
// partition by
527530
assertThat(QueryUtils.applySorting("select dense_rank() over (partition by age) from user u", sort))
528531
.isEqualTo("select dense_rank() over (partition by age) from user u order by u.age desc");
532+
// order by in over clause
529533
assertThat(QueryUtils.applySorting("select dense_rank() over (order by lastname) from user u", sort))
530534
.isEqualTo("select dense_rank() over (order by lastname) from user u order by u.age desc");
535+
// order by in over clause (additional spaces)
531536
assertThat(QueryUtils.applySorting("select dense_rank() over ( order by lastname ) from user u", sort))
532537
.isEqualTo("select dense_rank() over ( order by lastname ) from user u order by u.age desc");
538+
// order by in over clause + at the end
533539
assertThat(QueryUtils.applySorting("select dense_rank() over (order by lastname) from user u order by u.lastname", sort))
534540
.isEqualTo("select dense_rank() over (order by lastname) from user u order by u.lastname, u.age desc");
541+
// partition by + order by in over clause
535542
assertThat(QueryUtils.applySorting("select dense_rank() over (partition by active, age order by lastname) from user u", sort))
536543
.isEqualTo("select dense_rank() over (partition by active, age order by lastname) from user u order by u.age desc");
544+
// partition by + order by in over clause + order by at the end
537545
assertThat(QueryUtils.applySorting("select dense_rank() over (partition by active, age order by lastname) from user u order by active", sort))
538546
.isEqualTo("select dense_rank() over (partition by active, age order by lastname) from user u order by active, u.age desc");
547+
// partition by + order by in over clause + frame clause
539548
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))
540549
.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");
550+
// partition by + order by in over clause + frame clause + order by at the end
541551
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))
542552
.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");
553+
// order by in subselect (select expression)
554+
assertThat(QueryUtils.applySorting("select lastname, (select i.id from item i order by i.id limit 1) from user u", sort))
555+
.isEqualTo("select lastname, (select i.id from item i order by i.id limit 1) from user u order by u.age desc");
556+
// order by in subselect (select expression) + at the end
557+
assertThat(QueryUtils.applySorting("select lastname, (select i.id from item i order by 1 limit 1) from user u order by active", sort))
558+
.isEqualTo("select lastname, (select i.id from item i order by 1 limit 1) from user u order by active, u.age desc");
559+
// order by in subselect (from expression)
560+
assertThat(QueryUtils.applySorting("select * from (select * from user order by age desc limit 10) u", sort))
561+
.isEqualTo("select * from (select * from user order by age desc limit 10) u order by age desc");
562+
// order by in subselect (from expression) + at the end
563+
assertThat(QueryUtils.applySorting("select * from (select * from user order by 1, 2, 3 desc limit 10) u order by u.active asc", sort))
564+
.isEqualTo("select * from (select * from user order by 1, 2, 3 desc limit 10) u order by u.active asc, age desc");
543565
}
544566

545567
}

0 commit comments

Comments
 (0)