diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/Alter.java b/src/main/java/net/sf/jsqlparser/statement/alter/Alter.java index ebb1efafe2..b2183ad915 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/Alter.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/Alter.java @@ -12,7 +12,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.Optional; @@ -117,21 +116,34 @@ public String toString() { StringBuilder b = new StringBuilder(); appendTargetTo(b); - Iterator altIter = alterExpressions.iterator(); - - while (altIter.hasNext()) { - b.append(altIter.next().toString()); - - // Need to append whitespace after each ADD or DROP statement - // but not the last one - if (altIter.hasNext()) { - b.append(", "); + for (int i = 0; i < alterExpressions.size(); i++) { + AlterExpression action = alterExpressions.get(i); + if (i > 0) { + b.append(separatorBefore(action)); } + b.append(action); } return b.toString(); } + /** + * Whether the action is MySQL's PARTITION BY or REMOVE PARTITIONING, which may end the action + * list without a comma. + */ + public static boolean isTrailingPartitionOption(AlterExpression action) { + return action != null && (action.getOperation() == AlterOperation.PARTITION_BY + || action.getOperation() == AlterOperation.REMOVE_PARTITIONING); + } + + /** + * The separator written before an action that follows another: a comma, or a space before + * PARTITION BY and REMOVE PARTITIONING, which MySQL does not accept after a comma. + */ + public static String separatorBefore(AlterExpression action) { + return isTrailingPartitionOption(action) ? " " : ", "; + } + public Alter withTable(Table table) { this.setTable(table); return this; diff --git a/src/main/java/net/sf/jsqlparser/util/deparser/AlterDeParser.java b/src/main/java/net/sf/jsqlparser/util/deparser/AlterDeParser.java index a08e4f11ec..5c09eb9bec 100644 --- a/src/main/java/net/sf/jsqlparser/util/deparser/AlterDeParser.java +++ b/src/main/java/net/sf/jsqlparser/util/deparser/AlterDeParser.java @@ -17,7 +17,6 @@ import net.sf.jsqlparser.statement.alter.AlterExpressionPrimaryKey; import net.sf.jsqlparser.statement.alter.RelationAlterAction; import net.sf.jsqlparser.statement.select.PlainSelect; -import java.util.Iterator; import net.sf.jsqlparser.statement.create.table.IndexOptionSource; public class AlterDeParser extends AbstractDeParser { @@ -35,12 +34,13 @@ public AlterDeParser(StringBuilder buffer, ExpressionVisitor expr @Override public void deParse(Alter alter) { alter.appendTargetTo(builder); - for (Iterator iterator = alter.getAlterExpressions().iterator(); iterator - .hasNext();) { - deParseAction(iterator.next()); - if (iterator.hasNext()) { - builder.append(", "); + boolean first = true; + for (AlterExpression action : alter.getAlterExpressions()) { + if (!first) { + builder.append(Alter.separatorBefore(action)); } + deParseAction(action); + first = false; } } diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index 8420622293..d9700fc455 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -2515,6 +2515,15 @@ public class CCJSqlParser extends AbstractJSqlParser { || isKeywordAhead("MAX_ROWS") || isKeywordAhead("MIN_ROWS"); } + /** + * PARTITION BY or REMOVE PARTITIONING: the MySQL partition options that may end the ALTER TABLE + * action list without a comma. A column definition does not take them as column options. + */ + private boolean isTrailingPartitionOptionAhead() { + return getToken(1).kind == K_PARTITION && getToken(2).kind == K_BY + || getToken(1).kind == K_REMOVE && getToken(2).kind == K_PARTITIONING; + } + private boolean isMySqlTableOptionAction(AlterExpression action) { return action instanceof AlterExpressionTableOption && ((AlterExpressionTableOption) action).getStructuredTableOption() != null @@ -20177,7 +20186,8 @@ AlterExpression.ColumnDataType AlterExpressionColumnDataType(): useSetData=TypeChangePrefix() { withType = true; } ] ( LOOKAHEAD(2) dataType = ColDataType() )? ( LOOKAHEAD(2, { !(withType && getToken(1).kind == K_USING) - && getToken(1).kind != K_FIRST && !isKeywordAhead("AFTER") }) + && getToken(1).kind != K_FIRST && !isKeywordAhead("AFTER") + && !isTrailingPartitionOptionAhead() }) option = ColumnDefinitionOption() { columnOptions.add(option); } )* @@ -22219,6 +22229,16 @@ Alter AlterTable(boolean foreign): alterExp=AlterSharedTableOption() { alter.addAlterExpression(alterExp); } )* + // MySQL partition_options end the action list without a comma + [ + LOOKAHEAD({ isTrailingPartitionOptionAhead() }) + { + requireDdlSyntax(!Alter.isTrailingPartitionOption(alterExp), + "PARTITION BY and REMOVE PARTITIONING cannot follow each other"); + } + alterExp=AlterExpressionPartitionOp() { alter.addAlterExpression(alterExp); } + ] + { for (AlterExpression action : alter.getAlterExpressions()) { requireDdlSyntax(foreign || !(action instanceof AlterForeignDataOptions), "OPTIONS requires ALTER FOREIGN TABLE"); diff --git a/src/test/java/net/sf/jsqlparser/statement/alter/MySQLAlterTablePartitionTest.java b/src/test/java/net/sf/jsqlparser/statement/alter/MySQLAlterTablePartitionTest.java index b4ce3d7e02..1648373a31 100644 --- a/src/test/java/net/sf/jsqlparser/statement/alter/MySQLAlterTablePartitionTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/alter/MySQLAlterTablePartitionTest.java @@ -19,9 +19,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; +import java.util.function.Consumer; +import java.util.stream.Collectors; import java.util.stream.Stream; import net.sf.jsqlparser.JSQLParserException; import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect; +import net.sf.jsqlparser.parser.CCJSqlParser; import net.sf.jsqlparser.parser.CCJSqlParserUtil; import net.sf.jsqlparser.schema.Table; import net.sf.jsqlparser.statement.TableMaintenanceStatement.Modifier; @@ -29,6 +32,7 @@ import net.sf.jsqlparser.statement.create.table.TablePartitioning; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; /** MySQL 8.4 {@code ALTER TABLE} partition syntax verified against the MySQL manual. */ @@ -263,6 +267,77 @@ public void testPartitionByAst() throws JSQLParserException { assertEquals(partitioning.getPartitionDefinitions(), expression.getPartitionDefinitions()); } + @ParameterizedTest + @MethodSource("partitionOptionsAfterActions") + public void testPartitionOptionsEndTheActionListWithoutComma(String sql, + List operations) throws JSQLParserException { + // with the MySQL dialect and without a dialect + for (Consumer mode : List.>of( + parser -> parser.withDialect(Dialect.MYSQL), parser -> { + })) { + Alter alter = + assertInstanceOf(Alter.class, assertSqlCanBeParsedAndDeparsed(sql, true, mode)); + + assertEquals(operations, alter.getAlterExpressions().stream() + .map(AlterExpression::getOperation).collect(Collectors.toList())); + } + } + + static Stream partitionOptionsAfterActions() { + return Stream.of( + Arguments.of("ALTER TABLE t1 ADD COLUMN d INT PARTITION BY HASH (id) PARTITIONS 4", + List.of(AlterOperation.ADD, AlterOperation.PARTITION_BY)), + Arguments.of("ALTER TABLE t1 ENGINE = InnoDB REMOVE PARTITIONING", + List.of(AlterOperation.ENGINE, AlterOperation.REMOVE_PARTITIONING)), + Arguments.of("ALTER TABLE t1 ENGINE = InnoDB PARTITION BY KEY (id) PARTITIONS 2", + List.of(AlterOperation.ENGINE, AlterOperation.PARTITION_BY)), + Arguments.of("ALTER TABLE t1 ALGORITHM = INPLACE, ADD COLUMN x INT NOT NULL " + + "DEFAULT 0 PARTITION BY KEY (id) PARTITIONS 2", + List.of(AlterOperation.ALGORITHM, AlterOperation.ADD, + AlterOperation.PARTITION_BY)), + Arguments.of("ALTER TABLE t1 MODIFY COLUMN d BIGINT REMOVE PARTITIONING", + List.of(AlterOperation.MODIFY, AlterOperation.REMOVE_PARTITIONING)), + Arguments.of( + "ALTER TABLE t1 CHANGE COLUMN d e INT AFTER id PARTITION BY RANGE (id) " + + "(PARTITION p0 VALUES LESS THAN (10), " + + "PARTITION p1 VALUES LESS THAN MAXVALUE)", + List.of(AlterOperation.CHANGE, AlterOperation.PARTITION_BY)), + Arguments.of("ALTER TABLE t1 ADD INDEX i (a) PARTITION BY HASH (id)", + List.of(AlterOperation.ADD, AlterOperation.PARTITION_BY)), + Arguments.of("ALTER TABLE t1 DROP COLUMN c REMOVE PARTITIONING", + List.of(AlterOperation.DROP, AlterOperation.REMOVE_PARTITIONING))); + } + + @Test + public void testColumnDefinitionStopsBeforePartitionOptions() throws JSQLParserException { + Alter alter = (Alter) CCJSqlParserUtil.parse( + "ALTER TABLE t1 ADD COLUMN d INT PARTITION BY HASH (id) PARTITIONS 4", + parser -> parser.withDialect(Dialect.MYSQL)); + + AlterExpression add = alter.getAlterExpressions().get(0); + assertEquals("d INT", add.getColDataTypeList().get(0).toString()); + assertTrue(add.getColDataTypeList().get(0).getColumnOptions().isEmpty()); + AlterExpressionPartition partitionBy = + assertInstanceOf(AlterExpressionPartition.class, + alter.getAlterExpressions().get(1)); + assertEquals(TablePartitioning.Type.HASH, partitionBy.getPartitioning().getType()); + assertEquals(4L, partitionBy.getPartitioning().getPartitions()); + } + + @Test + public void testPartitionOptionsAreNotPrecededByComma() throws JSQLParserException { + // MySQL rejects a comma before PARTITION BY; the rendering drops it + Alter alter = (Alter) CCJSqlParserUtil.parse( + "ALTER TABLE t1 ADD COLUMN d INT, PARTITION BY HASH (id)"); + assertEquals("ALTER TABLE t1 ADD COLUMN d INT PARTITION BY HASH (id)", alter.toString()); + assertSqlCanBeParsedAndDeparsed(alter.toString(), true); + assertEquals(" ", Alter.separatorBefore(alter.getAlterExpressions().get(1))); + assertEquals(", ", Alter.separatorBefore(alter.getAlterExpressions().get(0))); + assertThrows(JSQLParserException.class, () -> CCJSqlParserUtil.parse( + "ALTER TABLE t1 PARTITION BY HASH (id) REMOVE PARTITIONING", + parser -> parser.withDialect(Dialect.MYSQL))); + } + @Test public void testProgrammaticConstruction() { AlterExpressionPartition expression = new AlterExpressionPartition()