From 583431de7890429891ba411be74b04e2c17fe40b Mon Sep 17 00:00:00 2001 From: minleejae Date: Sun, 11 Oct 2026 18:54:38 +0900 Subject: [PATCH] Separate ALTER key definitions from column drop targets Signed-off-by: minleejae --- .../statement/alter/AlterExpression.java | 212 ++++++++----- .../validation/validator/AlterValidator.java | 77 +++-- .../net/sf/jsqlparser/parser/JSqlParserCC.jjt | 11 +- src/site/sphinx/usage.rst | 51 +++ .../alter/AlterKeyAccessorsTest.java | 13 +- .../statement/alter/AlterKeyStateTest.java | 293 ++++++++++++++++++ .../validator/AlterValidatorTest.java | 46 +++ 7 files changed, 594 insertions(+), 109 deletions(-) create mode 100644 src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyStateTest.java diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java index a6881a702..353fe2297 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -55,9 +55,7 @@ public enum TableRenameKeyword { private List columnSetDefaultList; private List columnSetVisibilityList; - private List pkColumns; - private List ukColumns; - private String ukName; + private List dropColumns; private Index index = null; private Index oldIndex = null; private NamedConstraint constraint; @@ -65,8 +63,6 @@ public enum TableRenameKeyword { private String constraintName; private boolean usingIfExists; - private boolean uk; - private boolean ukTypeSpecified; private boolean useEqual; private List partitions; @@ -634,75 +630,144 @@ public void setUsingIfExists(boolean usingIfExists) { } /** - * Returns a snapshot of the rendered key elements for a structured PRIMARY_KEY, or the legacy - * list otherwise. Changing the snapshot does not change the constraint; use - * {@link #setPkColumns(List)} to replace its keys, or {@link KeyConstraint#getColumns()} for - * structured edits. + * Returns the stored key definition, or null for another definition. The returned object is + * shared with {@link #getConstraint()} and can be edited directly. + */ + public KeyConstraint getKeyConstraint() { + return constraint instanceof KeyConstraint ? (KeyConstraint) constraint : null; + } + + /** + * Returns the owned mutable column targets for parenthesized DROP, DROP UNIQUE or DROP FOREIGN + * KEY syntax, or null when omitted. Stored targets are independent of any ADD definition. + */ + public List getDropColumns() { + return dropColumns; + } + + /** Copies the supplied targets without changing the operation or any stored definition. */ + public void setDropColumns(List dropColumns) { + this.dropColumns = dropColumns == null ? null : new ArrayList<>(dropColumns); + } + + public AlterExpression withDropColumns(List dropColumns) { + setDropColumns(dropColumns); + return this; + } + + public AlterExpression addDropColumns(String... columns) { + return addDropColumns(Arrays.asList(columns)); + } + + public AlterExpression addDropColumns(Collection columns) { + if (dropColumns == null) { + dropColumns = new ArrayList<>(); + } + dropColumns.addAll(columns); + return this; + } + + /** + * Returns a rendered snapshot of the stored PRIMARY KEY's elements, or null when absent. For + * DROP, DROP UNIQUE and DROP FOREIGN KEY operations this legacy accessor instead returns a + * snapshot of {@link #getDropColumns()}. Snapshot edits never change the AST. New code should + * use {@link #getKeyConstraint()} or {@link #getDropColumns()} explicitly. */ public List getPkColumns() { + if (usesLegacyDropColumns()) { + return dropColumns == null ? null : new ArrayList<>(dropColumns); + } return hasKeyConstraint(ConstraintKind.PRIMARY_KEY) ? ((KeyConstraint) constraint).getColumnsNames() - : pkColumns; + : null; } /** - * Replaces a structured PRIMARY_KEY's elements using - * {@link KeyConstraint#setColumnsNames(List)}. The strings are not parsed and existing element - * attributes are not retained, even when the rendered SQL is unchanged. Use - * {@link KeyConstraint#setColumns(List)} to preserve structured elements. Without a matching - * constraint, stores the legacy list. + * Replaces PRIMARY KEY elements with opaque names, without parsing or retaining element + * attributes. A non-null list creates a definition when none exists; a competing definition + * must be replaced explicitly using {@link #setConstraint(NamedConstraint)}. Null clears only + * an existing PRIMARY KEY's elements and does not create a definition. With DROP, DROP UNIQUE + * or DROP FOREIGN KEY already selected, delegates to {@link #setDropColumns(List)} instead. Set + * the operation first for this legacy DROP alias; new builders should use setDropColumns. + * + * @throws IllegalStateException if a non-null list would replace another definition */ public void setPkColumns(List pkColumns) { - if (hasKeyConstraint(ConstraintKind.PRIMARY_KEY)) { - ((KeyConstraint) constraint).setColumnsNames(pkColumns); - this.pkColumns = null; - } else { - this.pkColumns = pkColumns; + if (usesLegacyDropColumns()) { + setDropColumns(pkColumns); + return; + } + KeyConstraint key = keyForUpdate(ConstraintKind.PRIMARY_KEY, pkColumns != null); + if (key != null) { + key.setColumnsNames(pkColumns); } } /** - * Returns a snapshot of the rendered key elements for a structured UNIQUE, or the legacy list - * otherwise. Changing the snapshot does not change the constraint; use - * {@link #setUkColumns(List)} to replace its keys, or {@link KeyConstraint#getColumns()} for - * structured edits. + * Returns a snapshot of the stored UNIQUE's elements, or null when absent. Changing the + * snapshot does not change the constraint; use {@link #setUkColumns(List)} to replace its keys, + * or {@link KeyConstraint#getColumns()} for structured edits. */ public List getUkColumns() { return hasKeyConstraint(ConstraintKind.UNIQUE) ? ((KeyConstraint) constraint).getColumnsNames() - : ukColumns; + : null; } /** * Replaces a structured UNIQUE's elements using {@link KeyConstraint#setColumnsNames(List)}. * The strings are not parsed and existing element attributes are not retained, even when the * rendered SQL is unchanged. Use {@link KeyConstraint#setColumns(List)} to preserve structured - * elements. Without a matching constraint, stores the legacy list. + * elements. A non-null list creates a UNIQUE definition only when none exists; a competing + * definition must be replaced explicitly. Null clears only an existing UNIQUE's elements. + * + * @throws IllegalStateException if a non-null list would replace another definition */ public void setUkColumns(List ukColumns) { - if (hasKeyConstraint(ConstraintKind.UNIQUE)) { - ((KeyConstraint) constraint).setColumnsNames(ukColumns); - this.ukColumns = null; - } else { - this.ukColumns = ukColumns; + KeyConstraint key = keyForUpdate(ConstraintKind.UNIQUE, ukColumns != null); + if (key != null) { + key.setColumnsNames(ukColumns); } } + private boolean usesLegacyDropColumns() { + return operation == AlterOperation.DROP || operation == AlterOperation.DROP_UNIQUE + || operation == AlterOperation.DROP_FOREIGN_KEY; + } + private boolean hasKeyConstraint(ConstraintKind kind) { return constraint instanceof KeyConstraint && constraint.getKind() == kind; } + private KeyConstraint keyForUpdate(ConstraintKind kind, boolean create) { + if (hasKeyConstraint(kind)) { + return (KeyConstraint) constraint; + } + if (!create) { + return null; + } + if (constraint != null || index != null || getFkColumns() != null) { + throw new IllegalStateException( + "Key setters cannot replace an existing definition; use setConstraint"); + } + KeyConstraint key = new KeyConstraint() + .withType(kind == ConstraintKind.PRIMARY_KEY ? "PRIMARY KEY" : "UNIQUE"); + setConstraint(key); + return key; + } + public String getUkName() { return hasKeyConstraint(ConstraintKind.UNIQUE) ? ((KeyConstraint) constraint).getIndexName() - : ukName; + : null; } + /** Uses the creation and replacement policy of {@link #setUkColumns(List)}. */ public void setUkName(String ukName) { - if (hasKeyConstraint(ConstraintKind.UNIQUE)) { - ((KeyConstraint) constraint).setIndexName(ukName); + KeyConstraint key = keyForUpdate(ConstraintKind.UNIQUE, ukName != null); + if (key != null) { + key.setIndexName(ukName); } - this.ukName = ukName; } public Index getIndex() { @@ -754,12 +819,6 @@ private ConstraintKind getAddedConstraintKind() { if (index != null) { return ConstraintKind.OTHER; } - if (pkColumns != null) { - return ConstraintKind.PRIMARY_KEY; - } - if (ukColumns != null) { - return ConstraintKind.UNIQUE; - } return ConstraintKind.OTHER; } @@ -921,20 +980,53 @@ public void setUseEqual(boolean useEqual) { } public boolean getUk() { - return uk; + return "KEY".equalsIgnoreCase(getUniqueIndexKeyword()); } + /** Selects KEY when true or INDEX when false on the stored UNIQUE definition. */ public void setUk(boolean uk) { - this.uk = uk; - this.ukTypeSpecified = true; + setUniqueIndexKeyword(keyForUpdate(ConstraintKind.UNIQUE, true), uk ? "KEY" : "INDEX"); } public boolean isUkTypeSpecified() { - return ukTypeSpecified; + return getUniqueIndexKeyword() != null; } + /** + * Removes the explicit KEY/INDEX spelling when false. When true, retains an existing spelling + * or selects INDEX. Clearing an absent UNIQUE definition does not create one. + */ public void setUkTypeSpecified(boolean ukTypeSpecified) { - this.ukTypeSpecified = ukTypeSpecified; + KeyConstraint key = keyForUpdate(ConstraintKind.UNIQUE, ukTypeSpecified); + if (key != null) { + String keyword = getUniqueIndexKeyword(); + setUniqueIndexKeyword(key, + ukTypeSpecified ? keyword == null ? "INDEX" : keyword : null); + } + } + + private String getUniqueIndexKeyword() { + KeyConstraint key = keyForUpdate(ConstraintKind.UNIQUE, false); + if (key == null) { + return null; + } + if (key.getIndexKeyword() != null) { + return key.getIndexKeyword(); + } + if ("UNIQUE KEY".equalsIgnoreCase(key.getType())) { + return "KEY"; + } + return "UNIQUE INDEX".equalsIgnoreCase(key.getType()) ? "INDEX" : null; + } + + private void setUniqueIndexKeyword(KeyConstraint key, String keyword) { + if (hasLegacyKeyConstraint() || "UNIQUE KEY".equalsIgnoreCase(key.getType()) + || "UNIQUE INDEX".equalsIgnoreCase(key.getType())) { + key.setType("UNIQUE" + (keyword == null ? "" : " " + keyword)); + key.setIndexKeyword(null); + } else { + key.setIndexKeyword(keyword); + } } public boolean isUseIfNotExists() { @@ -1146,7 +1238,7 @@ protected boolean isDropSpecialOperation() { case DROP_CHECK: return true; case DROP: - return columnName == null && pkColumns != null && !pkColumns.isEmpty(); + return columnName == null && dropColumns != null && !dropColumns.isEmpty(); default: return false; } @@ -1307,14 +1399,15 @@ protected void toStringDropSpecial(StringBuilder b) { b.append("DROP PRIMARY KEY "); break; case DROP_UNIQUE: - b.append("DROP UNIQUE (").append(PlainSelect.getStringList(pkColumns)).append(')'); + b.append("DROP UNIQUE (").append(PlainSelect.getStringList(dropColumns)) + .append(')'); break; case DROP_FOREIGN_KEY: b.append("DROP FOREIGN KEY "); if (constraintName != null) { b.append(constraintName); } else { - b.append('(').append(PlainSelect.getStringList(pkColumns)).append(')'); + b.append('(').append(PlainSelect.getStringList(dropColumns)).append(')'); } break; case DROP_CHECK: @@ -1322,7 +1415,7 @@ protected void toStringDropSpecial(StringBuilder b) { break; default: // Oracle Multi Column Drop - b.append("DROP (").append(PlainSelect.getStringList(pkColumns)).append(')'); + b.append("DROP (").append(PlainSelect.getStringList(dropColumns)).append(')'); break; } } @@ -1538,26 +1631,7 @@ private void toStringGeneral(StringBuilder b, Consumer columnPri } else if (constraint != null) { b.append(constraint); } else if (index != null) { - // The structured index is canonical. Legacy PK/UK fields may also be populated for - // source compatibility, but cannot represent names, expressions, or index options. b.append(index); - } else if (pkColumns != null) { - b.append("PRIMARY KEY (").append(PlainSelect.getStringList(pkColumns)).append(')'); - } else if (ukColumns != null) { - b.append("UNIQUE"); - if (ukName != null) { - if (isUkTypeSpecified()) { - if (getUk()) { - b.append(" KEY "); - } else { - b.append(" INDEX "); - } - } else { - b.append(" "); - } - b.append(ukName); - } - b.append(" (").append(PlainSelect.getStringList(ukColumns)).append(")"); } if (getConstraints() != null && !getConstraints().isEmpty()) { diff --git a/src/main/java/net/sf/jsqlparser/util/validation/validator/AlterValidator.java b/src/main/java/net/sf/jsqlparser/util/validation/validator/AlterValidator.java index 912d2aed8..0e6750f18 100644 --- a/src/main/java/net/sf/jsqlparser/util/validation/validator/AlterValidator.java +++ b/src/main/java/net/sf/jsqlparser/util/validation/validator/AlterValidator.java @@ -24,6 +24,8 @@ import net.sf.jsqlparser.statement.create.table.KeyElement; import net.sf.jsqlparser.statement.create.table.ConstraintUsingIndex; import net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint; +import net.sf.jsqlparser.statement.create.table.ConstraintKind; +import net.sf.jsqlparser.statement.create.table.KeyConstraint; import net.sf.jsqlparser.util.TableDefinitionTraversal; import net.sf.jsqlparser.util.validation.ValidationCapability; import net.sf.jsqlparser.util.validation.ValidationUtil; @@ -73,36 +75,11 @@ public void validate(Alter alter, AlterExpression e) { validateOptionalName(c, NamedObject.constraint, e.getConstraintName()); validateOptionalName(c, NamedObject.constraint, e.getNewConstraintName(), null, false, NamedObject.table); - if (e.getPkColumns() != null) { - validateOptionalColumnNames(c, e.getPkColumns()); + if (hasActiveDropColumns(e)) { + validateOptionalColumnNames(c, e.getDropColumns()); } - if (e.getUk()) { - validateName(c, NamedObject.uniqueConstraint, e.getUkName()); - validateOptionalColumnNames(c, e.getUkColumns(), NamedObject.uniqueConstraint); - } - - NamedConstraint constraint = e.getConstraint(); - if (constraint != null) { - validateOptionalName(c, NamedObject.constraint, constraint.getName(), null, false, - NamedObject.table); - if (constraint instanceof ForeignKeyConstraint) { - validateOptionalColumnNames(c, - ((ForeignKeyConstraint) constraint).getReferencedColumnNames()); - } - if (constraint instanceof ConstraintUsingIndex) { - validateName(c, NamedObject.index, - ((ConstraintUsingIndex) constraint).getExistingIndexName()); - } - if (constraint instanceof KeyColumnSource) { - KeyColumnSource keys = (KeyColumnSource) constraint; - if (keys.getColumns() != null) { - validateOptionalColumnNames(c, keys.getColumns().stream() - .filter(key -> !key.isExpression()).map(KeyElement::getColumnName) - .collect(toList()), NamedObject.constraint); - } - } - } + validateConstraint(c, e.getConstraint()); if (e.getIndex() != null) { validateName(c, NamedObject.index, e.getIndex().getName()); if (e.getIndex().getColumns() != null) { @@ -114,5 +91,49 @@ public void validate(Alter alter, AlterExpression e) { } } + private void validateConstraint(ValidationCapability capability, NamedConstraint constraint) { + if (constraint == null) { + return; + } + validateOptionalName(capability, NamedObject.constraint, constraint.getName(), null, false, + NamedObject.table); + if (constraint instanceof ForeignKeyConstraint) { + validateOptionalColumnNames(capability, + ((ForeignKeyConstraint) constraint).getReferencedColumnNames()); + } + if (constraint instanceof KeyConstraint && constraint.getKind() == ConstraintKind.UNIQUE) { + validateOptionalName(capability, NamedObject.uniqueConstraint, + ((KeyConstraint) constraint).getIndexName(), null, false, NamedObject.table); + } + if (constraint instanceof ConstraintUsingIndex) { + validateName(capability, NamedObject.index, + ((ConstraintUsingIndex) constraint).getExistingIndexName()); + } + if (constraint instanceof KeyColumnSource) { + KeyColumnSource keys = (KeyColumnSource) constraint; + if (keys.getColumns() != null) { + validateOptionalColumnNames(capability, keys.getColumns().stream() + .filter(key -> !key.isExpression()).map(KeyElement::getColumnName) + .collect(toList()), NamedObject.constraint); + } + } + } + + private boolean hasActiveDropColumns(AlterExpression expression) { + if (expression.getOperation() == null || expression.getDropColumns() == null) { + return false; + } + switch (expression.getOperation()) { + case DROP: + return expression.getColumnName() == null && !expression.getDropColumns().isEmpty(); + case DROP_UNIQUE: + return true; + case DROP_FOREIGN_KEY: + return expression.getConstraintName() == null; + default: + return false; + } + } + } diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index 50d98ed83..b3b299663 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -2055,9 +2055,6 @@ public class CCJSqlParser extends AbstractJSqlParser { alterExp.setConstraint(constraint); if (constraint instanceof KeyConstraint && constraint.getKind() == ConstraintKind.UNIQUE) { KeyConstraint key = (KeyConstraint) constraint; - alterExp.setUkName(key.getIndexName()); - alterExp.setUk(key.getType().toUpperCase(Locale.ROOT).contains("KEY")); - alterExp.setUkTypeSpecified(key.getIndexKeyword() != null); // Preserve generic ALTER's historical placement of USING and COMMENT options. if (!Dialect.MYSQL.name().equals(getAsString(Feature.dialect)) && !Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect))) { @@ -18074,7 +18071,7 @@ void AlterExpressionAddConstraint(AlterExpression alterExp): ) | ( - tk= (tk2= { alterExp.setUk(true); } | tk2=)? + tk= (tk2= | tk2=)? clustering=SqlServerIndexClustering() columnNames=ColumnsNamesList() { @@ -18138,7 +18135,7 @@ AlterExpression AlterExpressionDrop(): | ( // Oracle Multi Column Drop - columnNames=ColumnsNamesList() { alterExp.setPkColumns(columnNames); columnNames = null; } + columnNames=ColumnsNamesList() { alterExp.setDropColumns(columnNames); columnNames = null; } [ "INVALIDATE" { alterExp.addParameters("INVALIDATE"); } ] [ "CASCADE" { alterExp.addParameters("CASCADE"); } @@ -18167,7 +18164,7 @@ AlterExpression AlterExpressionDrop(): | ( tk= { alterExp.setOperation(AlterOperation.DROP_UNIQUE); } - columnNames=ColumnsNamesList() { alterExp.setPkColumns(columnNames); columnNames = null; } + columnNames=ColumnsNamesList() { alterExp.setDropColumns(columnNames); columnNames = null; } [ ( tk= | tk= ) { alterExp.addParameters(tk.image); } ] ) | @@ -18178,7 +18175,7 @@ AlterExpression AlterExpressionDrop(): | ( tk= tk2= { alterExp.setOperation(AlterOperation.DROP_FOREIGN_KEY); } - columnNames=ColumnsNamesList() { alterExp.setPkColumns(columnNames); columnNames = null; } + columnNames=ColumnsNamesList() { alterExp.setDropColumns(columnNames); columnNames = null; } [ ( tk= | tk= ) { alterExp.addParameters(tk.image); } ] ) | diff --git a/src/site/sphinx/usage.rst b/src/site/sphinx/usage.rst index 498a9de48..d7f298623 100644 --- a/src/site/sphinx/usage.rst +++ b/src/site/sphinx/usage.rst @@ -519,6 +519,57 @@ Constraints with a fixed renderer, such as CHECK, always report their actual constraint kind. Replacing an index with a table constraint requires replacing the AST node; changing an index's type string cannot change its Java type. +ALTER key definitions use ``getKeyConstraint()`` for typed access to the same +object returned by ``getConstraint()``. PRIMARY KEY and UNIQUE builder setters +also create this canonical definition; they no longer keep separate column, +name or KEY/INDEX state: + +.. code-block:: java + + Alter add = (Alter) CCJSqlParserUtil.parse( + "ALTER TABLE t ADD UNIQUE KEY uq (id)", + parser -> parser.withDialect(Dialect.MYSQL)); + KeyConstraint key = add.getAlterExpressions().get(0).getKeyConstraint(); + key.setColumns(List.of(new KeyElement("account_id"))); + key.setIndexName("uq_account"); + System.out.println(add); // ALTER TABLE t ADD UNIQUE KEY uq_account (account_id) + +Parenthesized deletion targets have their own ``getDropColumns()`` and +``setDropColumns()`` API. It covers multi-column ``DROP (...)``, ``DROP UNIQUE +(...)`` and the parser's legacy parenthesized ``DROP FOREIGN KEY (...)`` form. +Named constraint drops continue to use ``getConstraintName()``. No key +definition is manufactured for a drop: + +.. code-block:: java + + Alter drop = (Alter) CCJSqlParserUtil.parse("ALTER TABLE t DROP (old_a, old_b)"); + drop.getAlterExpressions().get(0).getDropColumns().set(0, "obsolete_a"); + System.out.println(drop); // ALTER TABLE t DROP (obsolete_a, old_b) + +``setDropColumns`` copies its input; ``getDropColumns`` returns the owned mutable +list. ``withDropColumns`` and ``addDropColumns`` also work before selecting the +operation. Stored drop targets and key definitions remain independent when an +operation changes. + +Migration of older ALTER builders: + +* ``getPkColumns()`` and ``getUkColumns()`` now consistently return snapshots, + including manually built keys. Use their setters/adders or edit + ``getKeyConstraint().getColumns()`` to change SQL. The string setters replace + elements without retaining expression or ordering attributes. +* The legacy PK accessor remains a DROP-target alias when DROP, DROP UNIQUE or + DROP FOREIGN KEY is already selected. Replace builders that set PK columns + before selecting DROP with ``withDropColumns(...)``; changing the operation + does not transfer a key definition into deletion targets. +* ``getUkName`` accesses the UNIQUE definition's index name; the constraint + symbol remains ``getKeyConstraint().getName()``. KEY/INDEX flags read that same + definition. ``setUkTypeSpecified(false)`` removes the spelling; re-enabling it + without a spelling selects INDEX. +* Non-null key setters create a definition only when none exists. They throw + ``IllegalStateException`` for a competing constraint or index; use + ``setConstraint`` for an explicit replacement. Reads and null clears never + create a definition or restore values from a replaced one. + Exclusion keys expose ``getExclusionOperatorReference()`` for structured access to an operator's ``schemaName``, ``name`` and ``useOperatorKeyword`` flag. In ``Dialect.POSTGRESQL``, both CREATE and ALTER accept ``WITH OPERATOR(pg_catalog.&&)``. diff --git a/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java index 1d4cfda83..45cb380a9 100644 --- a/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java @@ -152,17 +152,20 @@ void supportsAnIndexWithoutColumnsAndFluentPopulation(boolean primary) { } @Test - void retainsLegacyOnlyConstructionAndIgnoresUnrelatedIndexes() { + void legacyConstructionUsesASingleDefinitionAndRejectsUnrelatedIndexes() { AlterExpression action = new AlterExpression().withOperation(AlterOperation.ADD) .withPkColumns(new ArrayList<>(List.of("a"))); action.getPkColumns().add("b"); + assertEquals("ADD PRIMARY KEY (a)", action.toString()); + action.addPkColumns("b"); assertEquals("ADD PRIMARY KEY (a, b)", action.toString()); + assertSame(action.getConstraint(), action.getKeyConstraint()); Index unrelated = new Index().withType("INDEX").withColumnsNames(List.of("other")); action.setIndex(unrelated); - action.setPkColumns(List.of("pk")); - action.setUkColumns(List.of("uk")); - assertEquals(List.of("pk"), action.getPkColumns()); - assertEquals(List.of("uk"), action.getUkColumns()); + assertThrows(IllegalStateException.class, () -> action.setPkColumns(List.of("pk"))); + assertThrows(IllegalStateException.class, () -> action.setUkColumns(List.of("uk"))); + assertNull(action.getPkColumns()); + assertNull(action.getUkColumns()); assertEquals(List.of("other"), unrelated.getColumnsNames()); } } diff --git a/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyStateTest.java b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyStateTest.java new file mode 100644 index 000000000..176a5569a --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyStateTest.java @@ -0,0 +1,293 @@ +/*- + * #%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.alter; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.ArrayList; +import java.util.List; +import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.schema.Table; +import net.sf.jsqlparser.statement.create.table.CheckConstraint; +import net.sf.jsqlparser.statement.create.table.ConstraintKind; +import net.sf.jsqlparser.statement.create.table.ConstraintUsingIndex; +import net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint; +import net.sf.jsqlparser.statement.create.table.Index; +import net.sf.jsqlparser.statement.create.table.KeyConstraint; +import net.sf.jsqlparser.statement.create.table.KeyElement; +import net.sf.jsqlparser.statement.create.table.NamedConstraint; +import net.sf.jsqlparser.util.deparser.StatementDeParser; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.ValueSource; + +class AlterKeyStateTest { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void legacyBuildersCreateOneKeyAndExposeOnlySnapshots(boolean primary) throws Exception { + AlterExpression action = new AlterExpression(); + assertNull(action.getKeyConstraint()); + assertNull(action.getPkColumns()); + assertNull(action.getUkColumns()); + setColumns(action, primary, null); + action.setUkName(null); + action.setUkTypeSpecified(false); + assertNull(action.getConstraint()); + + List input = new ArrayList<>(List.of("\"key.name\"")); + setColumns(action, primary, input); + input.add("detached"); + KeyConstraint key = action.getKeyConstraint(); + assertSame(key, action.getConstraint()); + assertEquals(primary ? ConstraintKind.PRIMARY_KEY : ConstraintKind.UNIQUE, key.getKind()); + columns(action, primary).add("also_detached"); + assertEquals(List.of("\"key.name\""), key.getColumnsNames()); + assertNull(action.getOperation()); + action.setOperation(AlterOperation.ADD); + roundTrip(statement(action), Dialect.POSTGRESQL); + setColumns(action, primary, List.of("other")); + assertSame(key, action.getKeyConstraint()); + assertEquals(List.of("other"), key.getColumnsNames()); + roundTrip(statement(action), Dialect.POSTGRESQL); + setColumns(action, primary, null); + assertSame(key, action.getKeyConstraint()); + assertTrue(key.getColumns().isEmpty()); + } + + @Test + void parsedPrimaryKeyPreservesAttributesAndUsesTheSameEditableElements() throws Exception { + Alter statement = parse("ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (id) " + + "INCLUDE (payload) DEFERRABLE", Dialect.POSTGRESQL); + AlterExpression action = first(statement); + KeyConstraint key = action.getKeyConstraint(); + assertEquals(Boolean.TRUE, key.getConstraintAttributes().getDeferrable()); + assertEquals(List.of("payload"), key.getIncludeColumns()); + action.setPkColumns(List.of("\"new.id\"")); + assertEquals(List.of("\"new.id\""), key.getColumnsNames()); + KeyElement replacement = new KeyElement("final_id"); + key.setColumns(List.of(replacement)); + assertSame(replacement, action.getKeyConstraint().getColumns().get(0)); + assertEquals(List.of("final_id"), action.getPkColumns()); + assertEquals("ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (final_id) " + + "INCLUDE (payload) DEFERRABLE", statement.toString()); + roundTrip(statement, Dialect.POSTGRESQL); + } + + @ParameterizedTest + @ValueSource(strings = {"UNIQUE", "UNIQUE KEY", "UNIQUE INDEX"}) + void uniqueNameAndKeywordAliasesFollowTheStructuredDefinition(String type) throws Exception { + Alter statement = parse("ALTER TABLE t ADD " + type + " uq (id)", Dialect.MYSQL); + AlterExpression action = first(statement); + KeyConstraint key = action.getKeyConstraint(); + assertEquals(type.endsWith("KEY"), action.getUk()); + assertEquals(!"UNIQUE".equals(type), action.isUkTypeSpecified()); + key.setIndexName("typed_name"); + assertEquals("typed_name", action.getUkName()); + action.setUkName("legacy_name"); + assertEquals("legacy_name", key.getIndexName()); + action.setUk(true); + assertTrue(action.getUk()); + assertTrue(action.isUkTypeSpecified()); + assertEquals("ALTER TABLE t ADD UNIQUE KEY legacy_name (id)", statement.toString()); + roundTrip(statement, Dialect.MYSQL); + action.setUk(false); + assertFalse(action.getUk()); + assertEquals("ALTER TABLE t ADD UNIQUE INDEX legacy_name (id)", statement.toString()); + roundTrip(statement, Dialect.MYSQL); + action.setUkTypeSpecified(false); + assertFalse(action.isUkTypeSpecified()); + assertEquals("ALTER TABLE t ADD UNIQUE legacy_name (id)", statement.toString()); + roundTrip(statement, Dialect.MYSQL); + action.setUkTypeSpecified(true); + assertFalse(action.getUk()); + assertTrue(action.isUkTypeSpecified()); + roundTrip(statement, Dialect.MYSQL); + } + + @Test + void genericNamedUniqueKeywordEditsKeepTheLegacyHeaderInSync() throws Exception { + Alter statement = parse("ALTER TABLE t ADD CONSTRAINT UNIQUE KEY uq (id)", null); + AlterExpression action = first(statement); + action.setUk(false); + assertEquals("UNIQUE INDEX", action.getConstraintType()); + assertFalse(action.getUk()); + assertTrue(action.isUkTypeSpecified()); + roundTrip(statement, null); + action.setUkTypeSpecified(false); + assertEquals("UNIQUE", action.getConstraintType()); + assertFalse(action.isUkTypeSpecified()); + roundTrip(statement, null); + } + + @Test + void replacementsAndClearsCannotRestoreOldKeyNamesOrKeywords() { + AlterExpression action = new AlterExpression().withUkName("old_name").withUk(true) + .withUkColumns(List.of("old_column")); + KeyConstraint old = action.getKeyConstraint(); + KeyConstraint replacement = new KeyConstraint().withType("UNIQUE") + .withColumnsNames(List.of("new_column")); + action.setConstraint(replacement); + assertNull(action.getUkName()); + assertFalse(action.getUk()); + assertFalse(action.isUkTypeSpecified()); + assertEquals(List.of("new_column"), action.getUkColumns()); + assertEquals("old_name", old.getIndexName()); + action.setConstraint(null); + assertNull(action.getKeyConstraint()); + assertNull(action.getUkColumns()); + assertNull(action.getUkName()); + action.setUkName("fresh"); + assertNotSame(old, action.getKeyConstraint()); + assertNotSame(replacement, action.getKeyConstraint()); + assertNull(action.getKeyConstraint().getColumns()); + assertFalse(action.isUkTypeSpecified()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void settersRejectCompetingDefinitionsAndNullClearsLeaveThemIntact(boolean primary) { + List competitors = List.of( + new KeyConstraint().withType(primary ? "UNIQUE" : "PRIMARY KEY"), + new CheckConstraint(), new ForeignKeyConstraint(), new ConstraintUsingIndex()); + for (NamedConstraint competitor : competitors) { + AlterExpression action = new AlterExpression().withConstraint(competitor); + setColumns(action, primary, null); + assertSame(competitor, action.getConstraint()); + assertThrows(IllegalStateException.class, + () -> setColumns(action, primary, List.of("id"))); + assertSame(competitor, action.getConstraint()); + } + AlterExpression action = new AlterExpression(); + Index index = new Index().withType("INDEX").withName("ix"); + action.setIndex(index); + setColumns(action, primary, null); + assertThrows(IllegalStateException.class, + () -> setColumns(action, primary, List.of("id"))); + assertSame(index, action.getIndex()); + assertNull(action.getKeyConstraint()); + action.setIndex(null); + action.setFkColumns(List.of("fk_column")); + assertThrows(IllegalStateException.class, + () -> setColumns(action, primary, List.of("id"))); + assertEquals(List.of("fk_column"), action.getFkColumns()); + } + + @ParameterizedTest + @ValueSource(strings = {"DROP (\"a.b\", c) CASCADE CONSTRAINTS", + "DROP UNIQUE (\"a.b\", c)", "DROP FOREIGN KEY (\"a.b\", c)"}) + void parsedDropTargetsAreOwnedAndSeparateFromKeyDefinitions(String body) throws Exception { + Alter statement = parse("ALTER TABLE t " + body, null); + AlterExpression action = first(statement); + assertNull(action.getConstraint()); + assertNull(action.getKeyConstraint()); + assertEquals(List.of("\"a.b\"", "c"), action.getDropColumns()); + List snapshot = action.getPkColumns(); + snapshot.set(0, "detached"); + assertEquals("\"a.b\"", action.getDropColumns().get(0)); + action.getDropColumns().set(0, "\"new.name\""); + assertEquals(List.of("\"new.name\"", "c"), action.getPkColumns()); + roundTrip(statement, null); + List replacement = new ArrayList<>(List.of("other")); + action.setPkColumns(replacement); + replacement.add("detached"); + assertEquals(List.of("other"), action.getDropColumns()); + action.addDropColumns("next").addDropColumns(List.of("last")); + assertEquals(List.of("other", "next", "last"), action.getDropColumns()); + roundTrip(statement, null); + action.setPkColumns(null); + assertNull(action.getDropColumns()); + assertNull(action.getConstraint()); + } + + @ParameterizedTest + @EnumSource(value = AlterOperation.class, names = {"DROP", "DROP_UNIQUE", "DROP_FOREIGN_KEY"}) + void newDropBuilderDoesNotDependOnSetterOrder(AlterOperation operation) throws Exception { + AlterExpression action = new AlterExpression().withDropColumns(List.of("\"a.b\"")); + assertNull(action.getConstraint()); + assertNull(action.getPkColumns()); + action.setOperation(operation); + assertEquals(List.of("\"a.b\""), action.getPkColumns()); + roundTrip(statement(action), null); + } + + @Test + void changingOperationDoesNotConvertDefinitionsIntoDropTargets() throws Exception { + AlterExpression action = new AlterExpression().withPkColumns(List.of("id")) + .withDropColumns(List.of("old_id")).withOperation(AlterOperation.ADD); + KeyConstraint key = action.getKeyConstraint(); + assertEquals("ADD PRIMARY KEY (id)", action.toString()); + action.setOperation(AlterOperation.DROP); + assertEquals("DROP (old_id)", action.toString()); + assertEquals(List.of("old_id"), action.getPkColumns()); + action.setPkColumns(List.of("replacement")); + assertEquals(List.of("id"), key.getColumnsNames()); + assertEquals("DROP (replacement)", action.toString()); + action.setOperation(AlterOperation.ADD); + assertEquals("ADD PRIMARY KEY (id)", action.toString()); + assertEquals(List.of("replacement"), action.getDropColumns()); + roundTrip(statement(action), Dialect.POSTGRESQL); + action.setDropColumns(null); + assertSame(key, action.getKeyConstraint()); + assertNull(action.getDropColumns()); + } + + @Test + void namedAndPrimaryKeyDropsDoNotInventColumnTargets() throws Exception { + Alter statement = + parse("ALTER TABLE t DROP FOREIGN KEY fk, DROP PRIMARY KEY", Dialect.MYSQL); + for (AlterExpression action : statement.getAlterExpressions()) { + assertNull(action.getDropColumns()); + assertNull(action.getPkColumns()); + assertNull(action.getKeyConstraint()); + } + roundTrip(statement, Dialect.MYSQL); + } + + private static List columns(AlterExpression action, boolean primary) { + return primary ? action.getPkColumns() : action.getUkColumns(); + } + + private static void setColumns(AlterExpression action, boolean primary, List columns) { + if (primary) { + action.setPkColumns(columns); + } else { + action.setUkColumns(columns); + } + } + + private static Alter parse(String sql, Dialect dialect) throws JSQLParserException { + return (Alter) CCJSqlParserUtil.parse(sql, parser -> { + if (dialect != null) { + parser.withDialect(dialect); + } + }); + } + + private static AlterExpression first(Alter statement) { + return statement.getAlterExpressions().get(0); + } + + private static Alter statement(AlterExpression action) { + Alter statement = new Alter(); + statement.setTable(new Table("t")); + statement.addAlterExpression(action); + return statement; + } + + private static void roundTrip(Alter statement, Dialect dialect) throws JSQLParserException { + StringBuilder sql = new StringBuilder(); + statement.accept(new StatementDeParser(sql)); + assertEquals(statement.toString(), sql.toString()); + assertEquals(sql.toString(), parse(sql.toString(), dialect).toString()); + } +} diff --git a/src/test/java/net/sf/jsqlparser/util/validation/validator/AlterValidatorTest.java b/src/test/java/net/sf/jsqlparser/util/validation/validator/AlterValidatorTest.java index 3f8f98b10..1dae3bf75 100644 --- a/src/test/java/net/sf/jsqlparser/util/validation/validator/AlterValidatorTest.java +++ b/src/test/java/net/sf/jsqlparser/util/validation/validator/AlterValidatorTest.java @@ -25,12 +25,58 @@ import net.sf.jsqlparser.schema.Table; import net.sf.jsqlparser.statement.alter.Alter; import net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint; +import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect; +import net.sf.jsqlparser.util.validation.ValidationContext; import net.sf.jsqlparser.util.validation.ValidationTestAsserts; import net.sf.jsqlparser.util.validation.feature.DatabaseType; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class AlterValidatorTest extends ValidationTestAsserts { + @ParameterizedTest + @ValueSource(strings = {"ADD PRIMARY KEY (id)", "DROP FOREIGN KEY fk_name", "DROP COLUMN id"}) + void ignoresStoredDropColumnsWhenTheRenderedActionUsesAnotherTarget(String action) + throws JSQLParserException { + Alter alter = (Alter) CCJSqlParserUtil.parse("ALTER TABLE t " + action, + parser -> parser.withDialect(Dialect.MYSQL)); + alter.getAlterExpressions().get(0).setDropColumns(List.of("old_id")); + assertEquals("ALTER TABLE t " + action, alter.toString()); + List names = new ArrayList<>(); + DatabaseMetaDataValidation metadata = name -> { + names.add(name); + return !"old_id".equals(name.getFqn()); + }; + AlterValidator validator = new AlterValidator(); + validator.setContext(new ValidationContext().setCapabilities(List.of(metadata))); + validator.validate(alter); + assertTrue(validator.getValidationErrors().isEmpty(), + validator.getValidationErrors().toString()); + assertTrue(names.stream().noneMatch(name -> "old_id".equals(name.getFqn()))); + } + + @Test + void validatesKeyDefinitionsAndDropTargetsWithoutDuplicateKeyColumnChecks() { + List names = new ArrayList<>(); + DatabaseMetaDataValidation metadata = name -> { + names.add(name); + return !"uq".equals(name.getFqn()); + }; + List errors = new Validation(List.of(metadata), + "ALTER TABLE t ADD UNIQUE KEY uq (id)").validate(); + assertTrue(errors.isEmpty(), errors.toString()); + assertEquals(1, names.stream().filter(name -> "id".equals(name.getFqn())).count()); + assertEquals(1, names.stream().filter(name -> "uq".equals(name.getFqn())).count()); + + names.clear(); + errors = new Validation(List.of(metadata), "ALTER TABLE t DROP (old_a, old_b)").validate(); + assertTrue(errors.isEmpty(), errors.toString()); + assertEquals(List.of(new Named(NamedObject.table, "t"), + new Named(NamedObject.column, "old_a"), + new Named(NamedObject.column, "old_b")), names); + } + @Test void validatesForeignKeysFromTheirCurrentStructuredDefinition() throws JSQLParserException { Alter alter = (Alter) CCJSqlParserUtil.parse(