Skip to content

Commit e0446b9

Browse files
committed
Polishing.
See #2260 (c93aa25), #2500, #2518.
1 parent 7c90bf7 commit e0446b9

File tree

2 files changed

+57
-32
lines changed

2 files changed

+57
-32
lines changed

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

+25-19
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ private static String toJpaDirection(Order order) {
433433
@Nullable
434434
@Deprecated
435435
public static String detectAlias(String query) {
436+
436437
String alias = null;
437438
Matcher matcher = ALIAS_MATCH.matcher(removeSubqueries(query));
438439
while (matcher.find()) {
@@ -442,23 +443,25 @@ public static String detectAlias(String query) {
442443
}
443444

444445
/**
445-
* Remove subqueries from the query, in order to identify the correct alias
446-
* in order by clauses. If the entire query is surrounded by parenthesis, the
447-
* outermost parenthesis are not removed.
446+
* Remove subqueries from the query, in order to identify the correct alias in order by clauses. If the entire query
447+
* is surrounded by parenthesis, the outermost parenthesis are not removed.
448448
*
449449
* @param query
450450
* @return query with all subqueries removed.
451451
*/
452452
static String removeSubqueries(String query) {
453+
453454
if (!StringUtils.hasText(query)) {
454455
return query;
455456
}
456457

457-
final List<Integer> opens = new ArrayList<>();
458-
final List<Integer> closes = new ArrayList<>();
459-
final List<Boolean> closeMatches = new ArrayList<>();
460-
for (int i=0; i<query.length(); i++) {
461-
final char c = query.charAt(i);
458+
List<Integer> opens = new ArrayList<>();
459+
List<Integer> closes = new ArrayList<>();
460+
List<Boolean> closeMatches = new ArrayList<>();
461+
462+
for (int i = 0; i < query.length(); i++) {
463+
464+
char c = query.charAt(i);
462465
if (c == '(') {
463466
opens.add(i);
464467
} else if (c == ')') {
@@ -467,18 +470,19 @@ static String removeSubqueries(String query) {
467470
}
468471
}
469472

470-
final StringBuilder sb = new StringBuilder(query);
471-
final boolean startsWithParen = STARTS_WITH_PAREN.matcher(query).find();
472-
for (int i=opens.size()-1; i>=(startsWithParen?1:0); i--) {
473-
final Integer open = opens.get(i);
474-
final Integer close = findClose(open, closes, closeMatches) + 1;
473+
StringBuilder sb = new StringBuilder(query);
474+
boolean startsWithParen = STARTS_WITH_PAREN.matcher(query).find();
475+
for (int i = opens.size() - 1; i >= (startsWithParen ? 1 : 0); i--) {
475476

477+
Integer open = opens.get(i);
478+
Integer close = findClose(open, closes, closeMatches) + 1;
476479

477480
if (close > open) {
478-
final String subquery = sb.substring(open, close);
479-
final Matcher matcher = PARENS_TO_REMOVE.matcher(subquery);
481+
482+
String subquery = sb.substring(open, close);
483+
Matcher matcher = PARENS_TO_REMOVE.matcher(subquery);
480484
if (matcher.find()) {
481-
sb.replace(open, close, new String(new char[close-open]).replace('\0', ' '));
485+
sb.replace(open, close, new String(new char[close - open]).replace('\0', ' '));
482486
}
483487
}
484488
}
@@ -487,8 +491,10 @@ static String removeSubqueries(String query) {
487491
}
488492

489493
private static Integer findClose(final Integer open, final List<Integer> closes, final List<Boolean> closeMatches) {
490-
for (int i=0; i<closes.size(); i++) {
491-
final int close = closes.get(i);
494+
495+
for (int i = 0; i < closes.size(); i++) {
496+
497+
int close = closes.get(i);
492498
if (close > open && !closeMatches.get(i)) {
493499
closeMatches.set(i, Boolean.TRUE);
494500
return close;
@@ -595,7 +601,7 @@ public static String createCountQueryFor(String originalQuery, @Nullable String
595601
String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue;
596602

597603
String alias = QueryUtils.detectAlias(originalQuery);
598-
if("*".equals(variable) && alias != null) {
604+
if ("*".equals(variable) && alias != null) {
599605
replacement = alias;
600606
}
601607

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

+32-13
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,19 @@ void detectsAliasCorrectly() throws Exception {
122122
assertThat(detectAlias("select u from T05User u")).isEqualTo("u");
123123
assertThat(detectAlias("select u from User u where not exists (from User u2)")).isEqualTo("u");
124124
assertThat(detectAlias("(select u from User u where not exists (from User u2))")).isEqualTo("u");
125-
assertThat(detectAlias("(select u from User u where not exists ((from User u2 where not exists (from User u3))))")).isEqualTo("u");
126-
assertThat(detectAlias("from Foo f left join f.bar b with type(b) = BarChild where (f.id = (select max(f.id) from Foo f2 where type(f2) = FooChild) or 1 <> 1) and 1=1")).isEqualTo("f");
127-
assertThat(detectAlias("(from Foo f max(f) ((((select * from Foo f2 (from Foo f3) max(*)) (from Foo f4)) max(f5)) (f6)) (from Foo f7))")).isEqualTo("f");
125+
assertThat(detectAlias("(select u from User u where not exists ((from User u2 where not exists (from User u3))))"))
126+
.isEqualTo("u");
127+
assertThat(detectAlias(
128+
"from Foo f left join f.bar b with type(b) = BarChild where (f.id = (select max(f.id) from Foo f2 where type(f2) = FooChild) or 1 <> 1) and 1=1"))
129+
.isEqualTo("f");
130+
assertThat(detectAlias(
131+
"(from Foo f max(f) ((((select * from Foo f2 (from Foo f3) max(*)) (from Foo f4)) max(f5)) (f6)) (from Foo f7))"))
132+
.isEqualTo("f");
128133
}
129134

130135
@Test // GH-2260
131136
void testRemoveSubqueries() throws Exception {
137+
132138
// boundary conditions
133139
assertThat(removeSubqueries(null)).isNull();
134140
assertThat(removeSubqueries("")).isEmpty();
@@ -147,11 +153,19 @@ void testRemoveSubqueries() throws Exception {
147153
assertThat(removeSubqueries("select u from User u")).isEqualTo("select u from User u");
148154
assertThat(removeSubqueries("select u from com.acme.User u")).isEqualTo("select u from com.acme.User u");
149155
assertThat(removeSubqueries("select u from T05User u")).isEqualTo("select u from T05User u");
150-
assertThat(normalizeWhitespace(removeSubqueries("select u from User u where not exists (from User u2)"))).isEqualTo("select u from User u where not exists");
151-
assertThat(normalizeWhitespace(removeSubqueries("(select u from User u where not exists (from User u2))"))).isEqualTo("(select u from User u where not exists )");
152-
assertThat(normalizeWhitespace(removeSubqueries("select u from User u where not exists (from User u2 where not exists (from User u3))"))).isEqualTo("select u from User u where not exists");
153-
assertThat(normalizeWhitespace(removeSubqueries("select u from User u where not exists ((from User u2 where not exists (from User u3)))"))).isEqualTo("select u from User u where not exists ( )");
154-
assertThat(normalizeWhitespace(removeSubqueries("(select u from User u where not exists ((from User u2 where not exists (from User u3))))"))).isEqualTo("(select u from User u where not exists ( ))");
156+
assertThat(normalizeWhitespace(removeSubqueries("select u from User u where not exists (from User u2)")))
157+
.isEqualTo("select u from User u where not exists");
158+
assertThat(normalizeWhitespace(removeSubqueries("(select u from User u where not exists (from User u2))")))
159+
.isEqualTo("(select u from User u where not exists )");
160+
assertThat(normalizeWhitespace(
161+
removeSubqueries("select u from User u where not exists (from User u2 where not exists (from User u3))")))
162+
.isEqualTo("select u from User u where not exists");
163+
assertThat(normalizeWhitespace(
164+
removeSubqueries("select u from User u where not exists ((from User u2 where not exists (from User u3)))")))
165+
.isEqualTo("select u from User u where not exists ( )");
166+
assertThat(normalizeWhitespace(
167+
removeSubqueries("(select u from User u where not exists ((from User u2 where not exists (from User u3))))")))
168+
.isEqualTo("(select u from User u where not exists ( ))");
155169
}
156170

157171
private String normalizeWhitespace(String s) {
@@ -692,16 +706,21 @@ void countQueryUsesCorrectVariable() {
692706
String countQueryFor = createCountQueryFor("SELECT * FROM User WHERE created_at > $1");
693707
assertThat(countQueryFor).isEqualTo("select count(*) FROM User WHERE created_at > $1");
694708

695-
countQueryFor = createCountQueryFor("SELECT * FROM mytable WHERE nr = :number AND kon = :kon AND datum >= '2019-01-01'");
696-
assertThat(countQueryFor).isEqualTo("select count(*) FROM mytable WHERE nr = :number AND kon = :kon AND datum >= '2019-01-01'");
709+
countQueryFor = createCountQueryFor(
710+
"SELECT * FROM mytable WHERE nr = :number AND kon = :kon AND datum >= '2019-01-01'");
711+
assertThat(countQueryFor)
712+
.isEqualTo("select count(*) FROM mytable WHERE nr = :number AND kon = :kon AND datum >= '2019-01-01'");
697713

698714
countQueryFor = createCountQueryFor("SELECT * FROM context ORDER BY time");
699715
assertThat(countQueryFor).isEqualTo("select count(*) FROM context");
700716

701717
countQueryFor = createCountQueryFor("select * FROM users_statuses WHERE (user_created_at BETWEEN $1 AND $2)");
702-
assertThat(countQueryFor).isEqualTo("select count(*) FROM users_statuses WHERE (user_created_at BETWEEN $1 AND $2)");
718+
assertThat(countQueryFor)
719+
.isEqualTo("select count(*) FROM users_statuses WHERE (user_created_at BETWEEN $1 AND $2)");
703720

704-
countQueryFor = createCountQueryFor("SELECT * FROM users_statuses us WHERE (user_created_at BETWEEN :fromDate AND :toDate)");
705-
assertThat(countQueryFor).isEqualTo("select count(us) FROM users_statuses us WHERE (user_created_at BETWEEN :fromDate AND :toDate)");
721+
countQueryFor = createCountQueryFor(
722+
"SELECT * FROM users_statuses us WHERE (user_created_at BETWEEN :fromDate AND :toDate)");
723+
assertThat(countQueryFor)
724+
.isEqualTo("select count(us) FROM users_statuses us WHERE (user_created_at BETWEEN :fromDate AND :toDate)");
706725
}
707726
}

0 commit comments

Comments
 (0)