Skip to content

Commit 39e12ea

Browse files
gregturngregturn
committed
Introduce support for ordering by aliased columns.
If a projection of either an HQL or JPQL query is aliased, do NOT prefix the FROM clause's alias prefix to any relevant applied sorting. Same for function-based order by arguments. Resolves #2863. Related: #2626, #2322, #1655. Original pull request: #2865.
1 parent 3721097 commit 39e12ea

File tree

7 files changed

+306
-174
lines changed

7 files changed

+306
-174
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

+31-44
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() {
@@ -118,7 +121,7 @@ public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContex
118121
tokens.addAll(visit(ctx.queryOrder()));
119122
}
120123

121-
if (this.sort.isSorted()) {
124+
if (sort.isSorted()) {
122125

123126
if (ctx.queryOrder() != null) {
124127

@@ -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, 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(() -> 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(() -> 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(() -> primaryFromAlias));
356343
}
357344
}
358345

@@ -363,8 +350,8 @@ public List<JpaQueryParsingToken> visitSelectClause(HqlParser.SelectClauseContex
363350
}
364351

365352
if (!projectionProcessed && !isSubquery(ctx)) {
366-
this.projection = selectionListTokens;
367-
this.projectionProcessed = true;
353+
projection = selectionListTokens;
354+
projectionProcessed = true;
368355
}
369356

370357
return tokens;
@@ -373,7 +360,7 @@ public List<JpaQueryParsingToken> visitSelectClause(HqlParser.SelectClauseContex
373360
@Override
374361
public List<JpaQueryParsingToken> visitInstantiation(HqlParser.InstantiationContext ctx) {
375362

376-
this.hasConstructorExpression = true;
363+
hasConstructorExpression = true;
377364

378365
return super.visitInstantiation(ctx);
379366
}

‎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
*/

0 commit comments

Comments
 (0)