Skip to content

Commit ed115c0

Browse files
committed
feat(parser): support opt-in legacy MySQL GROUP BY ordering
Preserve ASC/DESC by grouping-list position and share AST/deparser rendering. Keep existing expression traversal and make appending grouping expressions preserve both the expression list and its directions. Fixes #1169.
1 parent eddb1fb commit ed115c0

9 files changed

Lines changed: 279 additions & 42 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,9 @@ out. `is()` answers "did the grammar prove it", `may()` answers "could it be rul
189189
guard uses `may()` and a dispatcher uses `is()`. Function volatility is not a syntactic property,
190190
so anything the caller has not declared pure stays unproven and is listed by name.
191191

192+
Legacy MySQL `GROUP BY ... ASC/DESC` is available with `Dialect.MYSQL` and the explicit
193+
`withLegacyMySqlGroupBy(true)` option; modern/default parsing keeps it disabled.
194+
192195
## Piped SQL
193196

194197
Support is progressing for Piped SQL, which writes queries in the order they actually

‎src/main/java/net/sf/jsqlparser/parser/AbstractJSqlParser.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ public P withUnsupportedStatements(boolean allowUnsupportedStatements) {
102102
return withFeature(Feature.allowUnsupportedStatements, allowUnsupportedStatements);
103103
}
104104

105+
/** Enables GROUP BY ASC/DESC for MySQL versions before 8.0.13. Requires MYSQL dialect. */
106+
public P withLegacyMySqlGroupBy(boolean enabled) {
107+
return withFeature(Feature.allowLegacyMySqlGroupBy, enabled);
108+
}
109+
105110
public P withTimeOut(long timeOutMillSeconds) {
106111
return withFeature(Feature.timeOut, timeOutMillSeconds);
107112
}

‎src/main/java/net/sf/jsqlparser/parser/feature/Feature.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ public enum Feature {
7575
* "GROUP BY"
7676
*/
7777
selectGroupBy,
78+
/** Explicit ASC/DESC on GROUP BY items in legacy MySQL. */
79+
selectGroupByOrdering,
7880
/**
7981
* "GROUPING SETS"
8082
*/
@@ -784,6 +786,9 @@ public enum Feature {
784786
*/
785787
allowPostgresSpecificSyntax(false),
786788

789+
/** Enables legacy GROUP BY ordering with the MYSQL dialect; disabled by default. */
790+
allowLegacyMySqlGroupBy(false),
791+
787792
// PERFORMANCE
788793

789794
/**

‎src/main/java/net/sf/jsqlparser/statement/select/GroupByElement.java‎

Lines changed: 78 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,19 @@
1616
import java.util.Collections;
1717
import java.util.List;
1818
import java.util.Optional;
19+
import java.util.Objects;
20+
import java.util.function.Consumer;
1921

2022
import net.sf.jsqlparser.expression.Expression;
2123
import net.sf.jsqlparser.expression.operators.relational.ExpressionList;
2224
import net.sf.jsqlparser.expression.operators.relational.ParenthesedExpressionList;
2325

2426
public class GroupByElement implements Serializable {
27+
public enum SortDirection {
28+
ASC, DESC
29+
}
30+
31+
private final List<SortDirection> groupBySortDirections = new ArrayList<>();
2532
private ExpressionList<Expression> groupByExpressions = new ExpressionList<>();
2633
private List<ExpressionList<Expression>> groupingSets = new ArrayList<>();
2734
// postgres rollup is an ExpressionList
@@ -45,6 +52,9 @@ public ExpressionList<Expression> getGroupByExpressions() {
4552
}
4653

4754
public void setGroupByExpressions(ExpressionList<Expression> groupByExpressions) {
55+
if (this.groupByExpressions != groupByExpressions) {
56+
groupBySortDirections.clear();
57+
}
4858
this.groupByExpressions = groupByExpressions;
4959
}
5060

@@ -68,35 +78,83 @@ public void addGroupingSet(ExpressionList<Expression> list) {
6878
this.groupingSets.add(list);
6979
}
7080

81+
/** Returns the explicit direction at a grouping-list position, or null if omitted. */
82+
public SortDirection getGroupBySortDirection(int index) {
83+
Objects.checkIndex(index, groupByExpressions.size());
84+
return index < groupBySortDirections.size() ? groupBySortDirections.get(index) : null;
85+
}
86+
87+
/** Directions belong to list positions. Replacing the expression list clears them. */
88+
public void setGroupBySortDirection(int index, SortDirection direction) {
89+
Objects.checkIndex(index, groupByExpressions.size());
90+
while (groupBySortDirections.size() <= index) {
91+
groupBySortDirections.add(null);
92+
}
93+
groupBySortDirections.set(index, direction);
94+
}
95+
96+
public boolean hasGroupBySortDirections() {
97+
if (groupByExpressions != null && !groupBySortDirections.isEmpty()) {
98+
for (int i = 0; i < groupByExpressions.size(); i++) {
99+
if (getGroupBySortDirection(i) != null) {
100+
return true;
101+
}
102+
}
103+
}
104+
return false;
105+
}
106+
71107
@Override
72-
@SuppressWarnings({"PMD.CyclomaticComplexity"})
73108
public String toString() {
74-
StringBuilder b = new StringBuilder();
75-
b.append("GROUP BY ");
109+
StringBuilder builder = new StringBuilder();
110+
appendTo(builder, builder::append, builder::append);
111+
return builder.toString();
112+
}
76113

114+
/** Shares clause layout while letting a deparser visit each expression. */
115+
public void appendTo(StringBuilder builder, Consumer<ExpressionList<?>> listRenderer,
116+
Consumer<Expression> expressionRenderer) {
117+
builder.append("GROUP BY ");
77118
if (groupByExpressions != null) {
78-
b.append(groupByExpressions);
119+
if (hasGroupBySortDirections()) {
120+
appendOrderedExpressions(builder, expressionRenderer);
121+
} else {
122+
listRenderer.accept(groupByExpressions);
123+
}
79124
}
80-
81-
int i = 0;
82125
if (!groupingSets.isEmpty()) {
83-
if (b.charAt(b.length() - 1) != ' ') {
84-
b.append(' ');
126+
if (builder.charAt(builder.length() - 1) != ' ') {
127+
builder.append(' ');
85128
}
86-
b.append("GROUPING SETS (");
87-
for (ExpressionList<?> expressionList : groupingSets) {
88-
b.append(i++ > 0 ? ", " : "").append(Select.getStringList(
89-
expressionList,
90-
true, expressionList instanceof ParenthesedExpressionList));
129+
builder.append("GROUPING SETS (");
130+
for (int i = 0; i < groupingSets.size(); i++) {
131+
builder.append(i > 0 ? ", " : "");
132+
listRenderer.accept(groupingSets.get(i));
91133
}
92-
b.append(")");
134+
builder.append(")");
93135
}
94-
95136
if (isMysqlWithRollup()) {
96-
b.append(" WITH ROLLUP");
137+
builder.append(" WITH ROLLUP");
97138
}
139+
}
98140

99-
return b.toString();
141+
private void appendOrderedExpressions(StringBuilder builder,
142+
Consumer<Expression> expressionRenderer) {
143+
boolean brackets = groupByExpressions instanceof ParenthesedExpressionList<?>;
144+
if (brackets) {
145+
builder.append('(');
146+
}
147+
for (int i = 0; i < groupByExpressions.size(); i++) {
148+
builder.append(i > 0 ? ", " : "");
149+
expressionRenderer.accept(groupByExpressions.get(i));
150+
SortDirection direction = getGroupBySortDirection(i);
151+
if (direction != null) {
152+
builder.append(' ').append(direction);
153+
}
154+
}
155+
if (brackets) {
156+
builder.append(')');
157+
}
100158
}
101159

102160
public GroupByElement withGroupByExpressions(ExpressionList<Expression> groupByExpressions) {
@@ -115,9 +173,9 @@ public GroupByElement addGroupByExpressions(Expression... groupByExpressions) {
115173

116174
public GroupByElement addGroupByExpressions(
117175
Collection<? extends Expression> groupByExpressions) {
118-
ExpressionList collection =
119-
Optional.ofNullable(getGroupByExpressions()).orElseGet(ExpressionList::new);
120-
Collections.addAll(collection, groupByExpressions);
176+
ExpressionList<Expression> collection =
177+
Optional.ofNullable(getGroupByExpressionList()).orElseGet(ExpressionList::new);
178+
collection.addAll(groupByExpressions);
121179
return this.withGroupByExpressions(collection);
122180
}
123181

‎src/main/java/net/sf/jsqlparser/util/deparser/GroupByDeParser.java‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,41 +10,24 @@
1010
package net.sf.jsqlparser.util.deparser;
1111

1212
import net.sf.jsqlparser.expression.ExpressionVisitor;
13-
import net.sf.jsqlparser.expression.operators.relational.ExpressionList;
1413
import net.sf.jsqlparser.statement.select.GroupByElement;
1514

1615
public class GroupByDeParser extends AbstractDeParser<GroupByElement> {
1716

1817
private final ExpressionListDeParser<?> expressionListDeParser;
18+
private final ExpressionVisitor<StringBuilder> expressionVisitor;
1919

2020
public GroupByDeParser(ExpressionVisitor<StringBuilder> expressionVisitor,
2121
StringBuilder buffer) {
2222
super(buffer);
23+
this.expressionVisitor = expressionVisitor;
2324
this.expressionListDeParser = new ExpressionListDeParser<>(expressionVisitor, buffer);
2425
this.builder = buffer;
2526
}
2627

2728
@Override
28-
@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.NPathComplexity"})
2929
public void deParse(GroupByElement groupBy) {
30-
builder.append("GROUP BY ");
31-
expressionListDeParser.deParse(groupBy.getGroupByExpressionList());
32-
33-
int i = 0;
34-
if (!groupBy.getGroupingSets().isEmpty()) {
35-
if (builder.charAt(builder.length() - 1) != ' ') {
36-
builder.append(' ');
37-
}
38-
builder.append("GROUPING SETS (");
39-
for (ExpressionList<?> expressionList : groupBy.getGroupingSets()) {
40-
builder.append(i++ > 0 ? ", " : "");
41-
expressionListDeParser.deParse(expressionList);
42-
}
43-
builder.append(")");
44-
}
45-
46-
if (groupBy.isMysqlWithRollup()) {
47-
builder.append(" WITH ROLLUP");
48-
}
30+
groupBy.appendTo(builder, expressionListDeParser::deParse,
31+
expression -> expression.accept(expressionVisitor, null));
4932
}
5033
}

‎src/main/java/net/sf/jsqlparser/util/validation/validator/GroupByValidator.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public void validate(GroupByElement groupBy) {
3131
public <S> Void visit(GroupByElement groupBy, S context) {
3232
for (ValidationCapability c : getCapabilities()) {
3333
validateFeature(c, Feature.selectGroupBy);
34+
if (groupBy.hasGroupBySortDirections()) {
35+
validateFeature(c, Feature.selectGroupByOrdering);
36+
}
3437
if (isNotEmpty(groupBy.getGroupingSets())) {
3538
validateFeature(c, Feature.selectGroupByGroupingSets);
3639
}

‎src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7596,7 +7596,13 @@ GroupByElement GroupByColumnReferences():
75967596
)
75977597
|
75987598
(
7599-
list = ExpressionList() { groupBy.setGroupByExpressions(list); }
7599+
(
7600+
LOOKAHEAD({ getAsBoolean(Feature.allowLegacyMySqlGroupBy)
7601+
&& "MYSQL".equals(getAsString(Feature.dialect)) })
7602+
LegacyMySqlGroupByExpressions(groupBy)
7603+
|
7604+
list = ExpressionList() { groupBy.setGroupByExpressions(list); }
7605+
)
76007606
(
76017607
LOOKAHEAD(2) <K_GROUPING> <K_SETS>
76027608
"("
@@ -7612,6 +7618,27 @@ GroupByElement GroupByColumnReferences():
76127618
}
76137619
}
76147620

7621+
void LegacyMySqlGroupByExpressions(GroupByElement groupBy):
7622+
{}
7623+
{
7624+
LegacyMySqlGroupByExpression(groupBy)
7625+
( LOOKAHEAD(2) "," LegacyMySqlGroupByExpression(groupBy) )*
7626+
}
7627+
7628+
void LegacyMySqlGroupByExpression(GroupByElement groupBy):
7629+
{
7630+
Expression expression;
7631+
Token direction;
7632+
}
7633+
{
7634+
expression = Expression() { groupBy.getGroupByExpressionList().add(expression); }
7635+
[ LOOKAHEAD(2) (direction=<K_ASC> | direction=<K_DESC>) {
7636+
groupBy.setGroupBySortDirection(groupBy.getGroupByExpressionList().size() - 1,
7637+
direction.kind == K_ASC ? GroupByElement.SortDirection.ASC
7638+
: GroupByElement.SortDirection.DESC);
7639+
} ]
7640+
}
7641+
76157642
ExpressionList<Expression> GroupingSet():
76167643
{
76177644
ExpressionList<Expression> list;

‎src/site/sphinx/usage.rst‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,3 +872,21 @@ References: `CREATE ROLE <https://www.postgresql.org/docs/18/sql-createrole.html
872872
`REVOKE <https://www.postgresql.org/docs/18/sql-revoke.html>`_,
873873
`ALTER DEFAULT PRIVILEGES <https://www.postgresql.org/docs/18/sql-alterdefaultprivileges.html>`_,
874874
`CREATE TRIGGER <https://www.postgresql.org/docs/18/sql-createtrigger.html>`_.
875+
876+
Legacy MySQL GROUP BY ordering
877+
==============================
878+
879+
MySQL before 8.0.13 accepted ``ASC`` and ``DESC`` on individual ``GROUP BY`` items.
880+
Select the existing ``MYSQL`` dialect and explicitly enable this legacy syntax:
881+
882+
.. code-block:: java
883+
884+
Statement statement = CCJSqlParserUtil.parse(
885+
"SELECT a FROM t GROUP BY a DESC",
886+
parser -> parser.withDialect(Dialect.MYSQL).withLegacyMySqlGroupBy(true));
887+
888+
The option is disabled by default and does not enable this syntax in other dialects.
889+
``GroupByElement`` keeps its existing expression list; ``getGroupBySortDirection(index)``
890+
returns each explicit direction, or null when omitted. Directions follow list positions;
891+
replacing the expression list clears them. Validators report the separate
892+
``selectGroupByOrdering`` feature, which is not enabled in the MySQL 8.0 capability.

0 commit comments

Comments
 (0)