Skip to content

Commit 8f8252e

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 8b58803 commit 8f8252e

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
@@ -761,7 +761,7 @@ public QueryTokenStream visitAggregate_expression(EqlParser.Aggregate_expression
761761
builder.append(QueryTokens.expression(ctx.DISTINCT()));
762762
}
763763

764-
builder.appendInline(visit(ctx.state_valued_path_expression()));
764+
builder.appendInline(visit(ctx.simple_select_expression()));
765765
builder.append(TOKEN_CLOSE_PAREN);
766766
} else if (ctx.COUNT() != null) {
767767

@@ -770,13 +770,7 @@ public QueryTokenStream visitAggregate_expression(EqlParser.Aggregate_expression
770770
if (ctx.DISTINCT() != null) {
771771
builder.append(QueryTokens.expression(ctx.DISTINCT()));
772772
}
773-
if (ctx.identification_variable() != null) {
774-
builder.appendInline(visit(ctx.identification_variable()));
775-
} else if (ctx.state_valued_path_expression() != null) {
776-
builder.appendInline(visit(ctx.state_valued_path_expression()));
777-
} else if (ctx.single_valued_object_path_expression() != null) {
778-
builder.appendInline(visit(ctx.single_valued_object_path_expression()));
779-
}
773+
builder.appendInline(visit(ctx.simple_select_expression()));
780774
builder.append(TOKEN_CLOSE_PAREN);
781775
} else if (ctx.function_invocation() != null) {
782776
builder.append(visit(ctx.function_invocation()));
@@ -2080,16 +2074,7 @@ public QueryTokenStream visitDatetime_part(EqlParser.Datetime_partContext ctx) {
20802074

20812075
@Override
20822076
public QueryTokenStream visitFunction_arg(EqlParser.Function_argContext ctx) {
2083-
2084-
if (ctx.literal() != null) {
2085-
return visit(ctx.literal());
2086-
} else if (ctx.state_valued_path_expression() != null) {
2087-
return visit(ctx.state_valued_path_expression());
2088-
} else if (ctx.input_parameter() != null) {
2089-
return visit(ctx.input_parameter());
2090-
} else {
2091-
return visit(ctx.scalar_expression());
2092-
}
2077+
return visit(ctx.simple_select_expression());
20932078
}
20942079

20952080
@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
@@ -695,7 +695,7 @@ public QueryTokenStream visitAggregate_expression(JpqlParser.Aggregate_expressio
695695
builder.append(QueryTokens.expression(ctx.DISTINCT()));
696696
}
697697

698-
builder.appendInline(visit(ctx.state_valued_path_expression()));
698+
builder.appendInline(visit(ctx.simple_select_expression()));
699699
builder.append(TOKEN_CLOSE_PAREN);
700700
} else if (ctx.COUNT() != null) {
701701

@@ -704,13 +704,8 @@ public QueryTokenStream visitAggregate_expression(JpqlParser.Aggregate_expressio
704704
if (ctx.DISTINCT() != null) {
705705
builder.append(QueryTokens.expression(ctx.DISTINCT()));
706706
}
707-
if (ctx.identification_variable() != null) {
708-
builder.appendInline(visit(ctx.identification_variable()));
709-
} else if (ctx.state_valued_path_expression() != null) {
710-
builder.appendInline(visit(ctx.state_valued_path_expression()));
711-
} else if (ctx.single_valued_object_path_expression() != null) {
712-
builder.appendInline(visit(ctx.single_valued_object_path_expression()));
713-
}
707+
708+
builder.appendInline(visit(ctx.simple_select_expression()));
714709
builder.append(TOKEN_CLOSE_PAREN);
715710
} else if (ctx.function_invocation() != null) {
716711
builder.append(visit(ctx.function_invocation()));
@@ -1975,16 +1970,7 @@ public QueryTokenStream visitDatetime_part(JpqlParser.Datetime_partContext ctx)
19751970

19761971
@Override
19771972
public QueryTokenStream visitFunction_arg(JpqlParser.Function_argContext ctx) {
1978-
1979-
if (ctx.literal() != null) {
1980-
return visit(ctx.literal());
1981-
} else if (ctx.state_valued_path_expression() != null) {
1982-
return visit(ctx.state_valued_path_expression());
1983-
} else if (ctx.input_parameter() != null) {
1984-
return visit(ctx.input_parameter());
1985-
} else {
1986-
return visit(ctx.scalar_expression());
1987-
}
1973+
return visit(ctx.simple_select_expression());
19881974
}
19891975

19901976
@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)