diff --git a/README.md b/README.md index 27202cacc..f2b028b7b 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,9 @@ out. `is()` answers "did the grammar prove it", `may()` answers "could it be rul guard uses `may()` and a dispatcher uses `is()`. Function volatility is not a syntactic property, so anything the caller has not declared pure stays unproven and is listed by name. +Legacy MySQL `GROUP BY ... ASC/DESC` is available with `Dialect.MYSQL` and the explicit +`withLegacyMySqlGroupBy(true)` option; modern/default parsing keeps it disabled. + ## Piped SQL Support is progressing for Piped SQL, which writes queries in the order they actually diff --git a/src/main/java/net/sf/jsqlparser/parser/AbstractJSqlParser.java b/src/main/java/net/sf/jsqlparser/parser/AbstractJSqlParser.java index 050401b59..2f4710a7e 100644 --- a/src/main/java/net/sf/jsqlparser/parser/AbstractJSqlParser.java +++ b/src/main/java/net/sf/jsqlparser/parser/AbstractJSqlParser.java @@ -102,6 +102,11 @@ public P withUnsupportedStatements(boolean allowUnsupportedStatements) { return withFeature(Feature.allowUnsupportedStatements, allowUnsupportedStatements); } + /** Enables GROUP BY ASC/DESC for MySQL versions before 8.0.13. Requires MYSQL dialect. */ + public P withLegacyMySqlGroupBy(boolean enabled) { + return withFeature(Feature.allowLegacyMySqlGroupBy, enabled); + } + public P withTimeOut(long timeOutMillSeconds) { return withFeature(Feature.timeOut, timeOutMillSeconds); } diff --git a/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java b/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java index 7659b2597..2b5c296b9 100644 --- a/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java +++ b/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java @@ -75,6 +75,8 @@ public enum Feature { * "GROUP BY" */ selectGroupBy, + /** Explicit ASC/DESC on GROUP BY items in legacy MySQL. */ + selectGroupByOrdering, /** * "GROUPING SETS" */ @@ -786,6 +788,9 @@ public enum Feature { */ allowPostgresSpecificSyntax(false), + /** Enables legacy GROUP BY ordering with the MYSQL dialect; disabled by default. */ + allowLegacyMySqlGroupBy(false), + // PERFORMANCE /** diff --git a/src/main/java/net/sf/jsqlparser/statement/select/GroupByElement.java b/src/main/java/net/sf/jsqlparser/statement/select/GroupByElement.java index 63b6b479d..b0ba784b7 100644 --- a/src/main/java/net/sf/jsqlparser/statement/select/GroupByElement.java +++ b/src/main/java/net/sf/jsqlparser/statement/select/GroupByElement.java @@ -16,12 +16,19 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Objects; +import java.util.function.Consumer; import net.sf.jsqlparser.expression.Expression; import net.sf.jsqlparser.expression.operators.relational.ExpressionList; import net.sf.jsqlparser.expression.operators.relational.ParenthesedExpressionList; public class GroupByElement implements Serializable { + public enum SortDirection { + ASC, DESC + } + + private final List groupBySortDirections = new ArrayList<>(); private ExpressionList groupByExpressions = new ExpressionList<>(); private List> groupingSets = new ArrayList<>(); // postgres rollup is an ExpressionList @@ -45,6 +52,9 @@ public ExpressionList getGroupByExpressions() { } public void setGroupByExpressions(ExpressionList groupByExpressions) { + if (this.groupByExpressions != groupByExpressions) { + groupBySortDirections.clear(); + } this.groupByExpressions = groupByExpressions; } @@ -68,35 +78,83 @@ public void addGroupingSet(ExpressionList list) { this.groupingSets.add(list); } + /** Returns the explicit direction at a grouping-list position, or null if omitted. */ + public SortDirection getGroupBySortDirection(int index) { + Objects.checkIndex(index, groupByExpressions.size()); + return index < groupBySortDirections.size() ? groupBySortDirections.get(index) : null; + } + + /** Directions belong to list positions. Replacing the expression list clears them. */ + public void setGroupBySortDirection(int index, SortDirection direction) { + Objects.checkIndex(index, groupByExpressions.size()); + while (groupBySortDirections.size() <= index) { + groupBySortDirections.add(null); + } + groupBySortDirections.set(index, direction); + } + + public boolean hasGroupBySortDirections() { + if (groupByExpressions != null && !groupBySortDirections.isEmpty()) { + for (int i = 0; i < groupByExpressions.size(); i++) { + if (getGroupBySortDirection(i) != null) { + return true; + } + } + } + return false; + } + @Override - @SuppressWarnings({"PMD.CyclomaticComplexity"}) public String toString() { - StringBuilder b = new StringBuilder(); - b.append("GROUP BY "); + StringBuilder builder = new StringBuilder(); + appendTo(builder, builder::append, builder::append); + return builder.toString(); + } + /** Shares clause layout while letting a deparser visit each expression. */ + public void appendTo(StringBuilder builder, Consumer> listRenderer, + Consumer expressionRenderer) { + builder.append("GROUP BY "); if (groupByExpressions != null) { - b.append(groupByExpressions); + if (hasGroupBySortDirections()) { + appendOrderedExpressions(builder, expressionRenderer); + } else { + listRenderer.accept(groupByExpressions); + } } - - int i = 0; if (!groupingSets.isEmpty()) { - if (b.charAt(b.length() - 1) != ' ') { - b.append(' '); + if (builder.charAt(builder.length() - 1) != ' ') { + builder.append(' '); } - b.append("GROUPING SETS ("); - for (ExpressionList expressionList : groupingSets) { - b.append(i++ > 0 ? ", " : "").append(Select.getStringList( - expressionList, - true, expressionList instanceof ParenthesedExpressionList)); + builder.append("GROUPING SETS ("); + for (int i = 0; i < groupingSets.size(); i++) { + builder.append(i > 0 ? ", " : ""); + listRenderer.accept(groupingSets.get(i)); } - b.append(")"); + builder.append(")"); } - if (isMysqlWithRollup()) { - b.append(" WITH ROLLUP"); + builder.append(" WITH ROLLUP"); } + } - return b.toString(); + private void appendOrderedExpressions(StringBuilder builder, + Consumer expressionRenderer) { + boolean brackets = groupByExpressions instanceof ParenthesedExpressionList; + if (brackets) { + builder.append('('); + } + for (int i = 0; i < groupByExpressions.size(); i++) { + builder.append(i > 0 ? ", " : ""); + expressionRenderer.accept(groupByExpressions.get(i)); + SortDirection direction = getGroupBySortDirection(i); + if (direction != null) { + builder.append(' ').append(direction); + } + } + if (brackets) { + builder.append(')'); + } } public GroupByElement withGroupByExpressions(ExpressionList groupByExpressions) { @@ -115,9 +173,9 @@ public GroupByElement addGroupByExpressions(Expression... groupByExpressions) { public GroupByElement addGroupByExpressions( Collection groupByExpressions) { - ExpressionList collection = - Optional.ofNullable(getGroupByExpressions()).orElseGet(ExpressionList::new); - Collections.addAll(collection, groupByExpressions); + ExpressionList collection = + Optional.ofNullable(getGroupByExpressionList()).orElseGet(ExpressionList::new); + collection.addAll(groupByExpressions); return this.withGroupByExpressions(collection); } diff --git a/src/main/java/net/sf/jsqlparser/util/deparser/GroupByDeParser.java b/src/main/java/net/sf/jsqlparser/util/deparser/GroupByDeParser.java index fc20bd4cd..86bcc0ca6 100644 --- a/src/main/java/net/sf/jsqlparser/util/deparser/GroupByDeParser.java +++ b/src/main/java/net/sf/jsqlparser/util/deparser/GroupByDeParser.java @@ -10,41 +10,24 @@ package net.sf.jsqlparser.util.deparser; import net.sf.jsqlparser.expression.ExpressionVisitor; -import net.sf.jsqlparser.expression.operators.relational.ExpressionList; import net.sf.jsqlparser.statement.select.GroupByElement; public class GroupByDeParser extends AbstractDeParser { private final ExpressionListDeParser expressionListDeParser; + private final ExpressionVisitor expressionVisitor; public GroupByDeParser(ExpressionVisitor expressionVisitor, StringBuilder buffer) { super(buffer); + this.expressionVisitor = expressionVisitor; this.expressionListDeParser = new ExpressionListDeParser<>(expressionVisitor, buffer); this.builder = buffer; } @Override - @SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.NPathComplexity"}) public void deParse(GroupByElement groupBy) { - builder.append("GROUP BY "); - expressionListDeParser.deParse(groupBy.getGroupByExpressionList()); - - int i = 0; - if (!groupBy.getGroupingSets().isEmpty()) { - if (builder.charAt(builder.length() - 1) != ' ') { - builder.append(' '); - } - builder.append("GROUPING SETS ("); - for (ExpressionList expressionList : groupBy.getGroupingSets()) { - builder.append(i++ > 0 ? ", " : ""); - expressionListDeParser.deParse(expressionList); - } - builder.append(")"); - } - - if (groupBy.isMysqlWithRollup()) { - builder.append(" WITH ROLLUP"); - } + groupBy.appendTo(builder, expressionListDeParser::deParse, + expression -> expression.accept(expressionVisitor, null)); } } diff --git a/src/main/java/net/sf/jsqlparser/util/validation/validator/GroupByValidator.java b/src/main/java/net/sf/jsqlparser/util/validation/validator/GroupByValidator.java index ee2d7155f..0d5ed5fe9 100644 --- a/src/main/java/net/sf/jsqlparser/util/validation/validator/GroupByValidator.java +++ b/src/main/java/net/sf/jsqlparser/util/validation/validator/GroupByValidator.java @@ -31,6 +31,9 @@ public void validate(GroupByElement groupBy) { public Void visit(GroupByElement groupBy, S context) { for (ValidationCapability c : getCapabilities()) { validateFeature(c, Feature.selectGroupBy); + if (groupBy.hasGroupBySortDirections()) { + validateFeature(c, Feature.selectGroupByOrdering); + } if (isNotEmpty(groupBy.getGroupingSets())) { validateFeature(c, Feature.selectGroupByGroupingSets); } diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index 99698d75b..0a71f4373 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -7727,7 +7727,13 @@ GroupByElement GroupByColumnReferences(): ) | ( - list = ExpressionList() { groupBy.setGroupByExpressions(list); } + ( + LOOKAHEAD({ getAsBoolean(Feature.allowLegacyMySqlGroupBy) + && "MYSQL".equals(getAsString(Feature.dialect)) }) + LegacyMySqlGroupByExpressions(groupBy) + | + list = ExpressionList() { groupBy.setGroupByExpressions(list); } + ) ( LOOKAHEAD(2) "(" @@ -7743,6 +7749,27 @@ GroupByElement GroupByColumnReferences(): } } +void LegacyMySqlGroupByExpressions(GroupByElement groupBy): +{} +{ + LegacyMySqlGroupByExpression(groupBy) + ( LOOKAHEAD(2) "," LegacyMySqlGroupByExpression(groupBy) )* +} + +void LegacyMySqlGroupByExpression(GroupByElement groupBy): +{ + Expression expression; + Token direction; +} +{ + expression = Expression() { groupBy.getGroupByExpressionList().add(expression); } + [ LOOKAHEAD(2) (direction= | direction=) { + groupBy.setGroupBySortDirection(groupBy.getGroupByExpressionList().size() - 1, + direction.kind == K_ASC ? GroupByElement.SortDirection.ASC + : GroupByElement.SortDirection.DESC); + } ] +} + ExpressionList GroupingSet(): { ExpressionList list; diff --git a/src/site/sphinx/usage.rst b/src/site/sphinx/usage.rst index 1f090e2bb..b8250cf12 100644 --- a/src/site/sphinx/usage.rst +++ b/src/site/sphinx/usage.rst @@ -919,3 +919,21 @@ handler bodies. Procedure side effects remain unknown; table discovery reports u procedure calls, and feature analysis remains conservative. This covers anonymous blocks with variable declarations, SQL statements, assignments, calls, nesting and handlers, not all PL/SQL declarations, loops, packages or procedure definitions. + +Legacy MySQL GROUP BY ordering +============================== + +MySQL before 8.0.13 accepted ``ASC`` and ``DESC`` on individual ``GROUP BY`` items. +Select the existing ``MYSQL`` dialect and explicitly enable this legacy syntax: + +.. code-block:: java + + Statement statement = CCJSqlParserUtil.parse( + "SELECT a FROM t GROUP BY a DESC", + parser -> parser.withDialect(Dialect.MYSQL).withLegacyMySqlGroupBy(true)); + +The option is disabled by default and does not enable this syntax in other dialects. +``GroupByElement`` keeps its existing expression list; ``getGroupBySortDirection(index)`` +returns each explicit direction, or null when omitted. Directions follow list positions; +replacing the expression list clears them. Validators report the separate +``selectGroupByOrdering`` feature, which is not enabled in the MySQL 8.0 capability. diff --git a/src/test/java/net/sf/jsqlparser/statement/select/LegacyMySqlGroupByTest.java b/src/test/java/net/sf/jsqlparser/statement/select/LegacyMySqlGroupByTest.java new file mode 100644 index 000000000..d9b91d067 --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/select/LegacyMySqlGroupByTest.java @@ -0,0 +1,135 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2026 JSQLParser + * %% + * Dual licensed under GNU LGPL 2.1 or Apache License 2.0 + * #L% + */ +package net.sf.jsqlparser.statement.select; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Consumer; +import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.expression.ExpressionVisitorAdapter; +import net.sf.jsqlparser.expression.operators.relational.ExpressionList; +import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect; +import net.sf.jsqlparser.parser.CCJSqlParser; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.schema.Column; +import net.sf.jsqlparser.util.deparser.ExpressionDeParser; +import net.sf.jsqlparser.util.deparser.GroupByDeParser; +import net.sf.jsqlparser.util.validation.ValidationContext; +import net.sf.jsqlparser.util.validation.feature.MySqlVersion; +import net.sf.jsqlparser.util.validation.validator.GroupByValidator; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static net.sf.jsqlparser.test.TestUtils.assertSqlCanBeParsedAndDeparsed; +import static org.junit.jupiter.api.Assertions.*; + +class LegacyMySqlGroupByTest { + private static final Consumer LEGACY = + parser -> parser.withDialect(Dialect.MYSQL).withLegacyMySqlGroupBy(true); + + @ParameterizedTest + @ValueSource(strings = { + "SELECT a FROM b GROUP BY c DESC", + "SELECT COUNT(*) FROM t GROUP BY a ASC, b, c DESC", + "SELECT COUNT(*) FROM t GROUP BY (a + b) DESC, LOWER(c) ASC WITH ROLLUP", + "SELECT COUNT(*) FROM t GROUP BY a DESC HAVING COUNT(*) > 1 ORDER BY a ASC", + "SELECT COUNT(*) FROM t GROUP BY a, b WITH ROLLUP", + "SELECT COUNT(*) FROM t GROUP BY (a, b)" + }) + void parsesAndDeparsesLegacyGrouping(String sql) throws Exception { + for (boolean complex : new boolean[] {false, true}) { + Consumer config = parser -> { + LEGACY.accept(parser); + parser.withAllowComplexParsing(complex); + }; + PlainSelect select = (PlainSelect) assertSqlCanBeParsedAndDeparsed(sql, true, config); + assertEquals(select.toString(), + CCJSqlParserUtil.parse(select.toString(), config).toString()); + } + } + + @Test + void requiresBothMysqlDialectAndExplicitOptIn() { + String sql = "SELECT a FROM b GROUP BY c DESC"; + assertThrows(JSQLParserException.class, () -> CCJSqlParserUtil.parse(sql)); + assertThrows(JSQLParserException.class, () -> CCJSqlParserUtil.parse(sql, + parser -> parser.withDialect(Dialect.MYSQL))); + assertThrows(JSQLParserException.class, () -> CCJSqlParserUtil.parse(sql, + parser -> parser.withLegacyMySqlGroupBy(true))); + for (Dialect dialect : Dialect.values()) { + if (dialect != Dialect.MYSQL) { + assertThrows(JSQLParserException.class, () -> CCJSqlParserUtil.parse(sql, + parser -> parser.withDialect(dialect).withLegacyMySqlGroupBy(true))); + } + } + } + + @Test + void keepsExpressionsVisibleAndDirectionsDistinct() throws Exception { + PlainSelect select = (PlainSelect) CCJSqlParserUtil.parse( + "SELECT COUNT(*) FROM t GROUP BY a ASC, a DESC, b", LEGACY); + GroupByElement group = select.getGroupBy(); + assertInstanceOf(Column.class, group.getGroupByExpressionList().get(0)); + assertEquals(GroupByElement.SortDirection.ASC, group.getGroupBySortDirection(0)); + assertEquals(GroupByElement.SortDirection.DESC, group.getGroupBySortDirection(1)); + assertNull(group.getGroupBySortDirection(2)); + List columns = new ArrayList<>(); + new ExpressionVisitorAdapter() { + @Override + public Void visit(Column column, S context) { + columns.add(column.getColumnName()); + return null; + } + }.visit(group, null); + assertEquals(List.of("a", "a", "b"), columns); + + StringBuilder output = new StringBuilder(); + ExpressionDeParser expressions = new ExpressionDeParser() { + @Override + public StringBuilder visit(Column column, S context) { + return getBuilder().append("renamed_").append(column.getColumnName()); + } + }; + expressions.setBuilder(output); + new GroupByDeParser(expressions, output).deParse(group); + assertEquals("GROUP BY renamed_a ASC, renamed_a DESC, renamed_b", output.toString()); + + group.addGroupByExpressions(new Column("extra")); + assertEquals("GROUP BY a ASC, a DESC, b, extra", group.toString()); + group.setGroupByExpressions(new ExpressionList<>(new Column("replacement"))); + assertFalse(group.hasGroupBySortDirections()); + assertEquals("GROUP BY replacement", group.toString()); + } + + @Test + void validatesOrderingSeparatelyFromOrdinaryGrouping() throws Exception { + PlainSelect select = (PlainSelect) CCJSqlParserUtil.parse( + "SELECT a FROM t GROUP BY a DESC", LEGACY); + GroupByValidator validator = new GroupByValidator<>(); + validator.setContext(new ValidationContext() + .setCapabilities(Collections.singleton(MySqlVersion.V8_0))); + validator.validate(select.getGroupBy()); + assertTrue(validator.getValidationErrors().values().stream() + .flatMap(errors -> errors.stream()) + .anyMatch(error -> error.getMessage().contains("selectGroupByOrdering"))); + } + + @ParameterizedTest + @ValueSource(strings = { + "SELECT a FROM t GROUP BY a DESC ASC", + "SELECT a FROM t GROUP BY a DESC,", + "SELECT a FROM t GROUP BY a NULLS FIRST" + }) + void rejectsMalformedOrdering(String sql) { + assertThrows(JSQLParserException.class, () -> CCJSqlParserUtil.parse(sql, LEGACY)); + } +}