Skip to content

Commit 041898e

Browse files
mp911demp911de
committed
Fix nested EQL and JPQL aggregation function argument grammar.
We now accept a wider range of function arguments instead of limiting to property paths. Closes #4013
1 parent 4471618 commit 041898e

File tree

7 files changed

+35
-48
lines changed

7 files changed

+35
-48
lines changed

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,8 @@ constructor_item
214214
;
215215

216216
aggregate_expression
217-
: (AVG | MAX | MIN | SUM) '(' (DISTINCT)? state_valued_path_expression ')'
218-
| COUNT '(' (DISTINCT)? (identification_variable | state_valued_path_expression | single_valued_object_path_expression) ')'
217+
: (AVG | MAX | MIN | SUM) '(' (DISTINCT)? simple_select_expression ')'
218+
| COUNT '(' (DISTINCT)? simple_select_expression ')'
219219
| function_invocation
220220
;
221221

@@ -583,10 +583,7 @@ datetime_part
583583
;
584584

585585
function_arg
586-
: literal
587-
| state_valued_path_expression
588-
| input_parameter
589-
| scalar_expression
586+
: simple_select_expression
590587
;
591588

592589
case_expression

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ constructor_item
208208
;
209209

210210
aggregate_expression
211-
: (AVG | MAX | MIN | SUM) '(' (DISTINCT)? state_valued_path_expression ')'
212-
| COUNT '(' (DISTINCT)? (identification_variable | state_valued_path_expression | single_valued_object_path_expression) ')'
211+
: (AVG | MAX | MIN | SUM) '(' (DISTINCT)? simple_select_expression ')'
212+
| COUNT '(' (DISTINCT)? simple_select_expression ')'
213213
| function_invocation
214214
;
215215

@@ -571,10 +571,7 @@ datetime_part
571571
;
572572

573573
function_arg
574-
: literal
575-
| state_valued_path_expression
576-
| input_parameter
577-
| scalar_expression
574+
: simple_select_expression
578575
;
579576

580577
case_expression

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

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ public QueryTokenStream visitAggregate_expression(EqlParser.Aggregate_expression
766766
builder.append(QueryTokens.expression(ctx.DISTINCT()));
767767
}
768768

769-
builder.appendInline(visit(ctx.state_valued_path_expression()));
769+
builder.appendInline(visit(ctx.simple_select_expression()));
770770
builder.append(TOKEN_CLOSE_PAREN);
771771
} else if (ctx.COUNT() != null) {
772772

@@ -775,13 +775,7 @@ public QueryTokenStream visitAggregate_expression(EqlParser.Aggregate_expression
775775
if (ctx.DISTINCT() != null) {
776776
builder.append(QueryTokens.expression(ctx.DISTINCT()));
777777
}
778-
if (ctx.identification_variable() != null) {
779-
builder.appendInline(visit(ctx.identification_variable()));
780-
} else if (ctx.state_valued_path_expression() != null) {
781-
builder.appendInline(visit(ctx.state_valued_path_expression()));
782-
} else if (ctx.single_valued_object_path_expression() != null) {
783-
builder.appendInline(visit(ctx.single_valued_object_path_expression()));
784-
}
778+
builder.appendInline(visit(ctx.simple_select_expression()));
785779
builder.append(TOKEN_CLOSE_PAREN);
786780
} else if (ctx.function_invocation() != null) {
787781
builder.append(visit(ctx.function_invocation()));
@@ -2085,16 +2079,7 @@ public QueryTokenStream visitDatetime_part(EqlParser.Datetime_partContext ctx) {
20852079

20862080
@Override
20872081
public QueryTokenStream visitFunction_arg(EqlParser.Function_argContext ctx) {
2088-
2089-
if (ctx.literal() != null) {
2090-
return visit(ctx.literal());
2091-
} else if (ctx.state_valued_path_expression() != null) {
2092-
return visit(ctx.state_valued_path_expression());
2093-
} else if (ctx.input_parameter() != null) {
2094-
return visit(ctx.input_parameter());
2095-
} else {
2096-
return visit(ctx.scalar_expression());
2097-
}
2082+
return visit(ctx.simple_select_expression());
20982083
}
20992084

21002085
@Override

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

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,7 @@ public QueryTokenStream visitAggregate_expression(JpqlParser.Aggregate_expressio
702702
builder.append(QueryTokens.expression(ctx.DISTINCT()));
703703
}
704704

705-
builder.appendInline(visit(ctx.state_valued_path_expression()));
705+
builder.appendInline(visit(ctx.simple_select_expression()));
706706
builder.append(TOKEN_CLOSE_PAREN);
707707
} else if (ctx.COUNT() != null) {
708708

@@ -711,13 +711,8 @@ public QueryTokenStream visitAggregate_expression(JpqlParser.Aggregate_expressio
711711
if (ctx.DISTINCT() != null) {
712712
builder.append(QueryTokens.expression(ctx.DISTINCT()));
713713
}
714-
if (ctx.identification_variable() != null) {
715-
builder.appendInline(visit(ctx.identification_variable()));
716-
} else if (ctx.state_valued_path_expression() != null) {
717-
builder.appendInline(visit(ctx.state_valued_path_expression()));
718-
} else if (ctx.single_valued_object_path_expression() != null) {
719-
builder.appendInline(visit(ctx.single_valued_object_path_expression()));
720-
}
714+
715+
builder.appendInline(visit(ctx.simple_select_expression()));
721716
builder.append(TOKEN_CLOSE_PAREN);
722717
} else if (ctx.function_invocation() != null) {
723718
builder.append(visit(ctx.function_invocation()));
@@ -1982,16 +1977,7 @@ public QueryTokenStream visitDatetime_part(JpqlParser.Datetime_partContext ctx)
19821977

19831978
@Override
19841979
public QueryTokenStream visitFunction_arg(JpqlParser.Function_argContext ctx) {
1985-
1986-
if (ctx.literal() != null) {
1987-
return visit(ctx.literal());
1988-
} else if (ctx.state_valued_path_expression() != null) {
1989-
return visit(ctx.state_valued_path_expression());
1990-
} else if (ctx.input_parameter() != null) {
1991-
return visit(ctx.input_parameter());
1992-
} else {
1993-
return visit(ctx.scalar_expression());
1994-
}
1980+
return visit(ctx.simple_select_expression());
19951981
}
19961982

19971983
@Override

‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryRendererTests.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,14 @@ void findOrdersThatHaveProductNamedByAParameter() {
952952
""");
953953
}
954954

955+
@Test // GH-4013
956+
void minMaxFunctionsShouldWork() {
957+
assertQuery("SELECT MAX(e.age), e.address.city FROM Employee e");
958+
assertQuery("SELECT MAX(1), e.address.city FROM Employee e");
959+
assertQuery("SELECT MAX(MIN(MOD(e.salary, 10))), e.address.city FROM Employee e");
960+
assertQuery("SELECT MIN(MOD(e.salary, 10)), e.address.city FROM Employee e");
961+
}
962+
955963
@Test // GH-2982
956964
void floorShouldBeValidEntityName() {
957965

‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryRendererTests.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,6 +1713,12 @@ void lnFunctionShouldWork() {
17131713
assertQuery("select ln(7.5) from Element a");
17141714
}
17151715

1716+
@Test // GH-4013
1717+
void minMaxFunctionsShouldWork() {
1718+
assertQuery("SELECT MAX(MIN(MOD(e.salary, 10))), e.address.city FROM Employee e");
1719+
assertQuery("SELECT MIN(MOD(e.salary, 10)), e.address.city FROM Employee e");
1720+
}
1721+
17161722
@Test // GH-2981
17171723
void cteWithClauseShouldWork() {
17181724

‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryRendererTests.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -953,6 +953,14 @@ void findOrdersThatHaveProductNamedByAParameter() {
953953
""");
954954
}
955955

956+
@Test // GH-4013
957+
void minMaxFunctionsShouldWork() {
958+
assertQuery("SELECT MAX(e.age), e.address.city FROM Employee e");
959+
assertQuery("SELECT MAX(1), e.address.city FROM Employee e");
960+
assertQuery("SELECT MAX(MIN(MOD(e.salary, 10))), e.address.city FROM Employee e");
961+
assertQuery("SELECT MIN(MOD(e.salary, 10)), e.address.city FROM Employee e");
962+
}
963+
956964
@Test // GH-2982
957965
void floorShouldBeValidEntityName() {
958966

0 commit comments

Comments
 (0)