Skip to content

Commit 0274b7b

Browse files
gregturngregturn
committed
Introduce support for ordering by aliased columns.
If a projection of either an HQL or JPQL query is aliased, applied sorting should NOT result in that order parameter having the primary FROM clause's alias prefix to it. Same goes for function-based order by arguments. Resolves #2863. Related: #2626, #2322. Original pull request: #2865.
1 parent 7339c0f commit 0274b7b

File tree

7 files changed

+298
-166
lines changed

7 files changed

+298
-166
lines changed

‎spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4

+7-7
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@ identification_variable_declaration
6868
;
6969

7070
range_variable_declaration
71-
: entity_name (AS)? identification_variable
71+
: entity_name AS? identification_variable
7272
;
7373

7474
join
75-
: join_spec join_association_path_expression (AS)? identification_variable (join_condition)?
75+
: join_spec join_association_path_expression AS? identification_variable (join_condition)?
7676
;
7777

7878
fetch_join
@@ -103,7 +103,7 @@ join_single_valued_path_expression
103103
;
104104

105105
collection_member_declaration
106-
: IN '(' collection_valued_path_expression ')' (AS)? identification_variable
106+
: IN '(' collection_valued_path_expression ')' AS? identification_variable
107107
;
108108

109109
qualified_identification_variable
@@ -160,7 +160,7 @@ collection_valued_path_expression
160160
;
161161

162162
update_clause
163-
: UPDATE entity_name ((AS)? identification_variable)? SET update_item (',' update_item)*
163+
: UPDATE entity_name (AS? identification_variable)? SET update_item (',' update_item)*
164164
;
165165

166166
update_item
@@ -174,15 +174,15 @@ new_value
174174
;
175175

176176
delete_clause
177-
: DELETE FROM entity_name ((AS)? identification_variable)?
177+
: DELETE FROM entity_name (AS? identification_variable)?
178178
;
179179

180180
select_clause
181181
: SELECT (DISTINCT)? select_item (',' select_item)*
182182
;
183183

184184
select_item
185-
: select_expression ((AS)? result_variable)?
185+
: select_expression (AS? result_variable)?
186186
;
187187

188188
select_expression
@@ -247,7 +247,7 @@ subquery_from_clause
247247

248248
subselect_identification_variable_declaration
249249
: identification_variable_declaration
250-
| derived_path_expression (AS)? identification_variable (join)*
250+
| derived_path_expression AS? identification_variable (join)*
251251
| derived_collection_member_declaration
252252
;
253253

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java

+27-40
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,15 @@ class HqlQueryTransformer extends HqlQueryRenderer {
4141

4242
private final @Nullable String countProjection;
4343

44-
private @Nullable String alias = null;
44+
private @Nullable String primaryFromAlias = null;
4545

4646
private List<JpaQueryParsingToken> projection = Collections.emptyList();
4747
private boolean projectionProcessed;
4848

4949
private boolean hasConstructorExpression = false;
5050

51+
private JpaQueryTransformerSupport transformerSupport;
52+
5153
HqlQueryTransformer() {
5254
this(Sort.unsorted(), false, null);
5355
}
@@ -67,11 +69,12 @@ private HqlQueryTransformer(Sort sort, boolean countQuery, @Nullable String coun
6769
this.sort = sort;
6870
this.countQuery = countQuery;
6971
this.countProjection = countProjection;
72+
this.transformerSupport = new JpaQueryTransformerSupport();
7073
}
7174

7275
@Nullable
7376
public String getAlias() {
74-
return this.alias;
77+
return this.primaryFromAlias;
7578
}
7679

7780
public List<JpaQueryParsingToken> getProjection() {
@@ -130,29 +133,7 @@ public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContex
130133
tokens.add(TOKEN_ORDER_BY);
131134
}
132135

133-
this.sort.forEach(order -> {
134-
135-
JpaQueryParserSupport.checkSortExpression(order);
136-
137-
if (order.isIgnoreCase()) {
138-
tokens.add(TOKEN_LOWER_FUNC);
139-
}
140-
tokens.add(new JpaQueryParsingToken(() -> {
141-
142-
if (order.getProperty().contains("(")) {
143-
return order.getProperty();
144-
}
145-
146-
return this.alias + "." + order.getProperty();
147-
}, true));
148-
if (order.isIgnoreCase()) {
149-
NOSPACE(tokens);
150-
tokens.add(TOKEN_CLOSE_PAREN);
151-
}
152-
tokens.add(order.isDescending() ? TOKEN_DESC : TOKEN_ASC);
153-
tokens.add(TOKEN_COMMA);
154-
});
155-
CLIP(tokens);
136+
tokens.addAll(transformerSupport.generateOrderByArguments(primaryFromAlias, this.sort));
156137
}
157138
} else {
158139

@@ -176,7 +157,7 @@ public List<JpaQueryParsingToken> visitFromQuery(HqlParser.FromQueryContext ctx)
176157
if (countProjection != null) {
177158
tokens.add(new JpaQueryParsingToken(countProjection));
178159
} else {
179-
tokens.add(new JpaQueryParsingToken(() -> this.alias, false));
160+
tokens.add(new JpaQueryParsingToken(() -> this.primaryFromAlias, false));
180161
}
181162

182163
tokens.add(TOKEN_CLOSE_PAREN);
@@ -240,8 +221,8 @@ public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
240221
if (ctx.variable() != null) {
241222
tokens.addAll(visit(ctx.variable()));
242223

243-
if (this.alias == null && !isSubquery(ctx)) {
244-
this.alias = tokens.get(tokens.size() - 1).getToken();
224+
if (primaryFromAlias == null && !isSubquery(ctx)) {
225+
primaryFromAlias = tokens.get(tokens.size() - 1).getToken();
245226
}
246227
} else {
247228

@@ -250,8 +231,8 @@ public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
250231
tokens.add(TOKEN_AS);
251232
tokens.add(TOKEN_DOUBLE_UNDERSCORE);
252233

253-
if (this.alias == null && !isSubquery(ctx)) {
254-
this.alias = TOKEN_DOUBLE_UNDERSCORE.getToken();
234+
if (primaryFromAlias == null && !isSubquery(ctx)) {
235+
primaryFromAlias = TOKEN_DOUBLE_UNDERSCORE.getToken();
255236
}
256237
}
257238
}
@@ -267,8 +248,8 @@ public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
267248
if (ctx.variable() != null) {
268249
tokens.addAll(visit(ctx.variable()));
269250

270-
if (this.alias == null && !isSubquery(ctx)) {
271-
this.alias = tokens.get(tokens.size() - 1).getToken();
251+
if (primaryFromAlias == null && !isSubquery(ctx)) {
252+
primaryFromAlias = tokens.get(tokens.size() - 1).getToken();
272253
}
273254
}
274255
}
@@ -302,16 +283,22 @@ public List<JpaQueryParsingToken> visitJoin(HqlParser.JoinContext ctx) {
302283
@Override
303284
public List<JpaQueryParsingToken> visitAlias(HqlParser.AliasContext ctx) {
304285

305-
List<JpaQueryParsingToken> tokens = newArrayList();
286+
List<JpaQueryParsingToken> tokens = super.visitAlias(ctx);
306287

307-
if (ctx.AS() != null) {
308-
tokens.add(new JpaQueryParsingToken(ctx.AS()));
288+
if (primaryFromAlias == null && !isSubquery(ctx)) {
289+
primaryFromAlias = tokens.get(tokens.size() - 1).getToken();
309290
}
310291

311-
tokens.addAll(visit(ctx.identifier()));
292+
return tokens;
293+
}
294+
295+
@Override
296+
public List<JpaQueryParsingToken> visitVariable(HqlParser.VariableContext ctx) {
297+
298+
List<JpaQueryParsingToken> tokens = super.visitVariable(ctx);
312299

313-
if (this.alias == null && !isSubquery(ctx)) {
314-
this.alias = tokens.get(tokens.size() - 1).getToken();
300+
if (ctx.identifier() != null) {
301+
transformerSupport.registerAlias(tokens.get(tokens.size() - 1).getToken());
315302
}
316303

317304
return tokens;
@@ -346,13 +333,13 @@ public List<JpaQueryParsingToken> visitSelectClause(HqlParser.SelectClauseContex
346333

347334
if (selectionListTokens.stream().anyMatch(hqlToken -> hqlToken.getToken().contains("new"))) {
348335
// constructor
349-
tokens.add(new JpaQueryParsingToken(() -> this.alias));
336+
tokens.add(new JpaQueryParsingToken(() -> this.primaryFromAlias));
350337
} else {
351338
// keep all the select items to distinct against
352339
tokens.addAll(selectionListTokens);
353340
}
354341
} else {
355-
tokens.add(new JpaQueryParsingToken(() -> this.alias));
342+
tokens.add(new JpaQueryParsingToken(() -> this.primaryFromAlias));
356343
}
357344
}
358345

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryParserSupport.java

-26
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,12 @@
1818
import static org.springframework.data.jpa.repository.query.JpaQueryParsingToken.*;
1919

2020
import java.util.List;
21-
import java.util.regex.Pattern;
2221

2322
import org.antlr.v4.runtime.Lexer;
2423
import org.antlr.v4.runtime.Parser;
2524
import org.antlr.v4.runtime.ParserRuleContext;
2625
import org.antlr.v4.runtime.atn.PredictionMode;
27-
import org.springframework.dao.InvalidDataAccessApiUsageException;
2826
import org.springframework.data.domain.Sort;
29-
import org.springframework.data.jpa.domain.JpaSort;
3027
import org.springframework.data.util.Lazy;
3128
import org.springframework.lang.Nullable;
3229

@@ -39,12 +36,6 @@
3936
*/
4037
abstract class JpaQueryParserSupport {
4138

42-
private static final Pattern PUNCTUATION_PATTERN = Pattern.compile(".*((?![._])[\\p{Punct}|\\s])");
43-
44-
private static final String UNSAFE_PROPERTY_REFERENCE = "Sort expression '%s' must only contain property references or "
45-
+ "aliases used in the select clause; If you really want to use something other than that for sorting, please use "
46-
+ "JpaSort.unsafe(…)";
47-
4839
private final ParseState state;
4940

5041
JpaQueryParserSupport(String query) {
@@ -177,23 +168,6 @@ protected abstract List<JpaQueryParsingToken> doCreateCountQuery(ParserRuleConte
177168

178169
protected abstract boolean doCheckForConstructor(ParserRuleContext parsedQuery);
179170

180-
/**
181-
* Check any given {@link JpaSort.JpaOrder#isUnsafe()} order for presence of at least one property offending the
182-
* {@link #PUNCTUATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.
183-
*
184-
* @param order
185-
*/
186-
static void checkSortExpression(Sort.Order order) {
187-
188-
if (order instanceof JpaSort.JpaOrder && ((JpaSort.JpaOrder) order).isUnsafe()) {
189-
return;
190-
}
191-
192-
if (PUNCTUATION_PATTERN.matcher(order.getProperty()).find()) {
193-
throw new InvalidDataAccessApiUsageException(String.format(UNSAFE_PROPERTY_REFERENCE, order));
194-
}
195-
}
196-
197171
/**
198172
* Parser state capturing the lazily-parsed parser context.
199173
*/

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryTransformerSupport.java

+134
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package org.springframework.data.jpa.repository.query;
2+
3+
import static org.springframework.data.jpa.repository.query.JpaQueryParsingToken.*;
4+
5+
import java.util.ArrayList;
6+
import java.util.HashSet;
7+
import java.util.List;
8+
import java.util.Set;
9+
import java.util.regex.Pattern;
10+
11+
import org.springframework.dao.InvalidDataAccessApiUsageException;
12+
import org.springframework.data.domain.Sort;
13+
import org.springframework.data.jpa.domain.JpaSort;
14+
import org.springframework.lang.Nullable;
15+
16+
/**
17+
* Transformational operations needed to support either {@link HqlQueryTransformer} or {@link JpqlQueryTransformer}.
18+
*
19+
* @author Greg Turnquist
20+
* @since 3.1
21+
*/
22+
class JpaQueryTransformerSupport {
23+
24+
private static final Pattern PUNCTUATION_PATTERN = Pattern.compile(".*((?![._])[\\p{Punct}|\\s])");
25+
26+
private static final String UNSAFE_PROPERTY_REFERENCE = "Sort expression '%s' must only contain property references or "
27+
+ "aliases used in the select clause; If you really want to use something other than that for sorting, please use "
28+
+ "JpaSort.unsafe(…)";
29+
30+
private Set<String> projectionAliases;
31+
32+
JpaQueryTransformerSupport() {
33+
this.projectionAliases = new HashSet<>();
34+
}
35+
36+
/**
37+
* Register an {@literal alias} so it can later be evaluated when applying {@link Sort}s.
38+
*
39+
* @param token
40+
*/
41+
void registerAlias(String token) {
42+
projectionAliases.add(token);
43+
}
44+
45+
/**
46+
* Using the primary {@literal FROM} clause's alias and a {@link Sort}, construct all the {@literal ORDER BY}
47+
* arguments.
48+
*
49+
* @param primaryFromAlias
50+
* @param sort
51+
* @return
52+
*/
53+
List<JpaQueryParsingToken> generateOrderByArguments(String primaryFromAlias, Sort sort) {
54+
55+
List<JpaQueryParsingToken> tokens = new ArrayList<>();
56+
57+
sort.forEach(order -> {
58+
59+
checkSortExpression(order);
60+
61+
if (order.isIgnoreCase()) {
62+
tokens.add(TOKEN_LOWER_FUNC);
63+
}
64+
65+
tokens.add(new JpaQueryParsingToken(() -> generateOrderByArgument(primaryFromAlias, order)));
66+
67+
if (order.isIgnoreCase()) {
68+
NOSPACE(tokens);
69+
tokens.add(TOKEN_CLOSE_PAREN);
70+
}
71+
tokens.add(order.isDescending() ? TOKEN_DESC : TOKEN_ASC);
72+
tokens.add(TOKEN_COMMA);
73+
});
74+
CLIP(tokens);
75+
76+
return tokens;
77+
}
78+
79+
/**
80+
* Check any given {@link JpaSort.JpaOrder#isUnsafe()} order for presence of at least one property offending the
81+
* {@link #PUNCTUATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.
82+
*
83+
* @param order
84+
*/
85+
private void checkSortExpression(Sort.Order order) {
86+
87+
if (order instanceof JpaSort.JpaOrder && ((JpaSort.JpaOrder) order).isUnsafe()) {
88+
return;
89+
}
90+
91+
if (PUNCTUATION_PATTERN.matcher(order.getProperty()).find()) {
92+
throw new InvalidDataAccessApiUsageException(String.format(UNSAFE_PROPERTY_REFERENCE, order));
93+
}
94+
}
95+
96+
/**
97+
* Using the {@code primaryFromAlias} and the {@link org.springframework.data.domain.Sort.Order}, construct a suitable
98+
* argument to be added to an {@literal ORDER BY} expression.
99+
*
100+
* @param primaryFromAlias
101+
* @param order
102+
* @return
103+
*/
104+
private String generateOrderByArgument(@Nullable String primaryFromAlias, Sort.Order order) {
105+
106+
if (shouldPrefixWithAlias(order)) {
107+
return primaryFromAlias + "." + order.getProperty();
108+
} else {
109+
return order.getProperty();
110+
}
111+
}
112+
113+
/**
114+
* Determine when an {@link org.springframework.data.domain.Sort.Order} parameter should be prefixed with the primary
115+
* FROM clause's alias.
116+
*
117+
* @param order
118+
* @return boolean whether or not to apply the primary FROM clause's alias as a prefix
119+
*/
120+
private boolean shouldPrefixWithAlias(Sort.Order order) {
121+
122+
// If the Sort contains a function
123+
if (order.getProperty().contains("(")) {
124+
return false;
125+
}
126+
127+
// If the Sort references an alias
128+
if (projectionAliases.contains(order.getProperty())) {
129+
return false;
130+
}
131+
132+
return true;
133+
}
134+
}

0 commit comments

Comments
 (0)