From dd8b5c35e407bff45fd0e9a5072968913e6ca891 Mon Sep 17 00:00:00 2001 From: minleejae Date: Sun, 11 Oct 2026 16:28:36 +0900 Subject: [PATCH 1/2] Use one foreign-key state in ALTER TABLE Delegate legacy FK accessors to the structured constraint, remove copied grammar state and the fallback renderer, and validate the current reference. Preserve quoted name parts and protect existing non-FK definitions. This follow-up is based on PR #2747 at 9be36363. Signed-off-by: minleejae --- .../statement/alter/AlterExpression.java | 182 +++++++---- .../validation/validator/AlterValidator.java | 11 +- .../net/sf/jsqlparser/parser/JSqlParserCC.jjt | 15 - src/site/sphinx/usage.rst | 32 ++ .../alter/AlterForeignKeyStateTest.java | 306 ++++++++++++++++++ .../validator/AlterValidatorTest.java | 28 ++ 6 files changed, 482 insertions(+), 92 deletions(-) create mode 100644 src/test/java/net/sf/jsqlparser/statement/alter/AlterForeignKeyStateTest.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 45fa8ad19..9b8cb69d5 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -14,10 +14,8 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; -import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; import net.sf.jsqlparser.expression.Expression; @@ -27,6 +25,7 @@ import net.sf.jsqlparser.statement.ReferentialAction.Type; import net.sf.jsqlparser.statement.create.table.ColDataType; import net.sf.jsqlparser.statement.create.table.ColumnDefinition; +import net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint; import net.sf.jsqlparser.statement.create.table.Index; import net.sf.jsqlparser.statement.create.table.NamedConstraint; import net.sf.jsqlparser.statement.create.table.KeyConstraint; @@ -42,7 +41,6 @@ public enum TableRenameKeyword { NONE, TO, AS } - private final Set referentialActions = new LinkedHashSet<>(2); private AlterOperation operation; private String optionalSpecifier; private Table newTable; @@ -67,33 +65,6 @@ public enum TableRenameKeyword { private String constraintName; private boolean usingIfExists; - /** - * @deprecated Use {@link #setConstraint(NamedConstraint)} with a - * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. - */ - @Deprecated - private List fkColumns; - - /** - * @deprecated Use {@link #setConstraint(NamedConstraint)} with a - * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. - */ - @Deprecated - private String fkSourceSchema; - - /** - * @deprecated Use {@link #setConstraint(NamedConstraint)} with a - * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. - */ - @Deprecated - private String fkSourceTable; - - /** - * @deprecated Use {@link #setConstraint(NamedConstraint)} with a - * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. - */ - @Deprecated - private List fkSourceColumns; private boolean uk; private boolean ukTypeSpecified; private boolean useEqual; @@ -183,16 +154,20 @@ public void hasColumns(boolean hasColumns) { */ @Deprecated public String getFkSourceSchema() { - return fkSourceSchema; + Table table = getForeignKeyTable(); + return table == null ? null : table.getSchemaName(); } /** + * Updates only the referenced schema identifier, preserving its quoting and the table's other + * name parts. Uses the creation and replacement policy of {@link #setFkColumns(List)}. + * * @deprecated Use {@link #setConstraint(NamedConstraint)} with a * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. */ @Deprecated public void setFkSourceSchema(String fkSourceSchema) { - this.fkSourceSchema = fkSourceSchema; + setForeignKeyTablePart(fkSourceSchema, 1); } public String getCommentText() { @@ -228,8 +203,13 @@ public void setOptionalSpecifier(String optionalSpecifier) { } /** + * Updates the stored foreign key's referential action without replacing its object or column + * list. A null action removes it. Uses the creation and replacement policy of + * {@link #setFkColumns(List)}. + * * @param type * @param action + * @throws IllegalStateException if a non-null action would replace a different definition * @deprecated Standalone FK fields are deprecated. Use a * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} via * {@link #setConstraint(NamedConstraint)} instead. @@ -258,28 +238,27 @@ public void removeReferentialAction(Type type) { } /** + * Returns the action owned by the stored foreign key, or null when absent. Changes to the + * returned object are reflected in the constraint. + * * @param type * @return * @deprecated Standalone FK fields are deprecated. */ @Deprecated public ReferentialAction getReferentialAction(Type type) { - return referentialActions.stream() - .filter(ra -> type.equals(ra.getType())) - .findFirst() - .orElse(null); + ForeignKeyConstraint foreignKey = getForeignKeyConstraint(); + return foreignKey == null ? null : foreignKey.getReferentialAction(type); } private void setReferentialAction(Type type, Action action, boolean set) { - ReferentialAction found = getReferentialAction(type); - if (set) { - if (found == null) { - referentialActions.add(new ReferentialAction(type, action)); + ForeignKeyConstraint foreignKey = foreignKeyForUpdate(set && action != null); + if (foreignKey != null) { + if (set && action != null) { + foreignKey.setReferentialAction(type, action); } else { - found.setAction(action); + foreignKey.removeReferentialAction(type); } - } else if (found != null) { - referentialActions.remove(found); } } @@ -344,21 +323,41 @@ public void setOnDeleteSetNull(boolean onDeleteSetNull) { } /** - * @deprecated Use {@link #getConstraint()} with - * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. + * Returns a rendered snapshot of the stored foreign key's elements, or null when absent. + * Changing the snapshot does not change the constraint; use {@link #setFkColumns(List)} to + * replace its keys, or {@link ForeignKeyConstraint#getColumns()} for structured edits. + * + * @deprecated Use {@link #getConstraint()} with a {@link ForeignKeyConstraint} instead. */ @Deprecated public List getFkColumns() { - return fkColumns; + ForeignKeyConstraint foreignKey = getForeignKeyConstraint(); + return foreignKey == null || foreignKey.getColumns() == null ? null + : foreignKey.getColumnsNames(); } /** + * Replaces the stored foreign key's elements with opaque names without parsing or retaining + * element attributes. A non-null value creates a foreign key when no definition exists, but + * never replaces a different constraint or an ordinary index; use + * {@link #setConstraint(NamedConstraint)} to replace a definition explicitly. A null value + * clears existing foreign-key elements without creating a constraint. Other actions and their + * rendering precedence are unchanged. + * + * @throws IllegalStateException if a non-null value would replace a different definition * @deprecated Use {@link #setConstraint(NamedConstraint)} with a * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. */ @Deprecated public void setFkColumns(List fkColumns) { - this.fkColumns = fkColumns; + ForeignKeyConstraint foreignKey = foreignKeyForUpdate(fkColumns != null); + if (foreignKey != null) { + if (fkColumns == null) { + foreignKey.setColumns(null); + } else { + foreignKey.setColumnsNames(fkColumns); + } + } } /** @@ -367,16 +366,62 @@ public void setFkColumns(List fkColumns) { */ @Deprecated public String getFkSourceTable() { - return fkSourceTable; + Table table = getForeignKeyTable(); + return table == null ? null : table.getName(); } /** + * Updates only the referenced table identifier, preserving its quoting and the table's other + * name parts. Uses the creation and replacement policy of {@link #setFkColumns(List)}. + * * @deprecated Use {@link #setConstraint(NamedConstraint)} with a * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. */ @Deprecated public void setFkSourceTable(String fkSourceTable) { - this.fkSourceTable = fkSourceTable; + setForeignKeyTablePart(fkSourceTable, 0); + } + + private ForeignKeyConstraint getForeignKeyConstraint() { + return constraint instanceof ForeignKeyConstraint ? (ForeignKeyConstraint) constraint + : null; + } + + private ForeignKeyConstraint foreignKeyForUpdate(boolean create) { + ForeignKeyConstraint foreignKey = getForeignKeyConstraint(); + if (foreignKey == null && create) { + if (constraint != null || index != null || pkColumns != null || ukColumns != null) { + throw new IllegalStateException( + "Foreign-key setters cannot replace an existing definition; use setConstraint"); + } + foreignKey = new ForeignKeyConstraint(); + setConstraint(foreignKey); + } + return foreignKey; + } + + private Table getForeignKeyTable() { + ForeignKeyConstraint foreignKey = getForeignKeyConstraint(); + return foreignKey == null ? null : foreignKey.getTable(); + } + + private void setForeignKeyTablePart(String value, int partIndex) { + ForeignKeyConstraint foreignKey = foreignKeyForUpdate(value != null); + if (foreignKey == null || foreignKey.getTable() == null && value == null) { + return; + } + if (foreignKey.getTable() == null) { + foreignKey.setTable(new Table()); + } + Table table = foreignKey.getTable(); + // Table stores identifier parts in name-first order. Updating raw parts avoids splitting + // quoted identifiers containing dots and leaves the existing Table object attached. + List parts = new ArrayList<>(table.getNameParts()); + while (parts.size() <= partIndex) { + parts.add(null); + } + parts.set(partIndex, value); + table.setNameParts(parts); } public List getColDataTypeList() { @@ -456,21 +501,29 @@ public List getColumnSetVisibilityList() { } /** - * @deprecated Use {@link #getConstraint()} with - * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. + * Returns the referenced-column list owned by the stored foreign key, or null when absent. + * Changes to the returned list are reflected in the constraint. + * + * @deprecated Use {@link #getConstraint()} with a {@link ForeignKeyConstraint} instead. */ @Deprecated public List getFkSourceColumns() { - return fkSourceColumns; + ForeignKeyConstraint foreignKey = getForeignKeyConstraint(); + return foreignKey == null ? null : foreignKey.getReferencedColumnNames(); } /** + * Uses the creation and replacement policy of {@link #setFkColumns(List)}. + * * @deprecated Use {@link #setConstraint(NamedConstraint)} with a * {@link net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint} instead. */ @Deprecated public void setFkSourceColumns(List fkSourceColumns) { - this.fkSourceColumns = fkSourceColumns; + ForeignKeyConstraint foreignKey = foreignKeyForUpdate(fkSourceColumns != null); + if (foreignKey != null) { + foreignKey.setReferencedColumnNames(fkSourceColumns); + } } public String getNewTableName() { @@ -677,7 +730,7 @@ private ConstraintKind getAddedConstraintKind() { if (ukColumns != null) { return ConstraintKind.UNIQUE; } - return fkColumns != null ? ConstraintKind.FOREIGN_KEY : ConstraintKind.OTHER; + return ConstraintKind.OTHER; } private boolean hasColumnOrNamedTarget() { @@ -1451,7 +1504,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/FK fields may also be populated for + // 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) { @@ -1471,21 +1524,6 @@ private void toStringGeneral(StringBuilder b, Consumer columnPri b.append(ukName); } b.append(" (").append(PlainSelect.getStringList(ukColumns)).append(")"); - } else if (fkColumns != null) { - // @deprecated path - kept for backward compatibility when ForeignKeyConstraint is not - // set - b.append("FOREIGN KEY (") - .append(PlainSelect.getStringList(fkColumns)) - .append(") REFERENCES ") - .append( - fkSourceSchema != null && fkSourceSchema.trim().length() > 0 - ? fkSourceSchema + "." - : "") - .append(fkSourceTable) - .append(" (") - .append(PlainSelect.getStringList(fkSourceColumns)) - .append(")"); - referentialActions.forEach(b::append); } if (getConstraints() != null && !getConstraints().isEmpty()) { @@ -1574,7 +1612,7 @@ public AlterExpression withFkColumns(List fkColumns) { @Deprecated public AlterExpression withFkSourceSchema(String fkSourceSchema) { - this.setFkSourceTable(fkSourceSchema); + this.setFkSourceSchema(fkSourceSchema); return this; } @@ -1643,6 +1681,7 @@ public AlterExpression addUkColumns(Collection ukColumns) { return this.withUkColumns(collection); } + /** Adds strings using the replacement semantics of {@link #setFkColumns(List)}. */ @Deprecated public AlterExpression addFkColumns(String... fkColumns) { List collection = Optional.ofNullable(getFkColumns()).orElseGet(ArrayList::new); @@ -1650,6 +1689,7 @@ public AlterExpression addFkColumns(String... fkColumns) { return this.withFkColumns(collection); } + /** Adds strings using the replacement semantics of {@link #setFkColumns(List)}. */ @Deprecated public AlterExpression addFkColumns(Collection fkColumns) { List collection = Optional.ofNullable(getFkColumns()).orElseGet(ArrayList::new); 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 bdf63bf52..912d2aed8 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 @@ -23,6 +23,7 @@ import net.sf.jsqlparser.statement.create.table.KeyColumnSource; 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.util.TableDefinitionTraversal; import net.sf.jsqlparser.util.validation.ValidationCapability; import net.sf.jsqlparser.util.validation.ValidationUtil; @@ -76,12 +77,6 @@ public void validate(Alter alter, AlterExpression e) { validateOptionalColumnNames(c, e.getPkColumns()); } - if (e.getFkColumns() != null) { - validateName(c, NamedObject.table, e.getFkSourceTable()); - validateOptionalColumnNames(c, e.getFkColumns()); - validateOptionalColumnNames(c, e.getFkSourceColumns()); - } - if (e.getUk()) { validateName(c, NamedObject.uniqueConstraint, e.getUkName()); validateOptionalColumnNames(c, e.getUkColumns(), NamedObject.uniqueConstraint); @@ -91,6 +86,10 @@ public void validate(Alter alter, AlterExpression e) { 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()); diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index 6a82cb3bb..1efd04b9d 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -2072,21 +2072,6 @@ public class CCJSqlParser extends AbstractJSqlParser { } } } - if (constraint instanceof ForeignKeyConstraint) { - ForeignKeyConstraint foreignKey = (ForeignKeyConstraint) constraint; - alterExp.setFkColumns(foreignKey.getColumnsNames()); - if (foreignKey.getTable() != null) { - alterExp.setFkSourceSchema(foreignKey.getTable().getSchemaName()); - alterExp.setFkSourceTable(foreignKey.getTable().getName()); - } - alterExp.setFkSourceColumns(foreignKey.getReferencedColumnNames()); - for (ReferentialAction.Type type : ReferentialAction.Type.values()) { - ReferentialAction action = foreignKey.getReferentialAction(type); - if (action != null) { - alterExp.setReferentialAction(action.getType(), action.getAction()); - } - } - } } private void requireNoDropIndexOwner(Table owner) throws ParseException { diff --git a/src/site/sphinx/usage.rst b/src/site/sphinx/usage.rst index 218d6dcf7..ed6a99b74 100644 --- a/src/site/sphinx/usage.rst +++ b/src/site/sphinx/usage.rst @@ -428,6 +428,38 @@ index nodes for PRIMARY KEY/UNIQUE constraints. PostgreSQL ``USING INDEX`` retains the existing index name on ``ConstraintUsingIndex`` rather than creating key elements or an index definition. +An ALTER foreign key owns one ``ForeignKeyConstraint`` and its mutable +``ForeignKeyReference``. The legacy ``getFk*``/``setFk*`` accessors and +referential-action helpers delegate to these same nodes, so edits through +either API affect both SQL renderers. + +.. code-block:: java + + Alter alter = (Alter) CCJSqlParserUtil.parse( + "ALTER TABLE child ADD FOREIGN KEY (parent_id) REFERENCES parent (id)"); + AlterExpression action = alter.getAlterExpressions().get(0); + ForeignKeyConstraint foreignKey = (ForeignKeyConstraint) action.getConstraint(); + foreignKey.getReference().getTable().setSchemaName("archive"); + action.setFkSourceTable("parent_history"); + // Both APIs now refer to archive.parent_history. + foreignKey.setReferentialAction(ReferentialAction.Type.DELETE, + ReferentialAction.Action.CASCADE); + +New code should use the structured constraint API. ``getFkColumns()`` returns +a rendered snapshot, like ``getPkColumns()`` and ``getUkColumns()``; use +``setFkColumns()``/``addFkColumns()`` to replace or append names, or edit +``foreignKey.getColumns()`` directly. Referenced-column lists and +referential-action objects are live objects on the shared reference. + +A non-null legacy FK setter creates a foreign-key definition when none is +present. It rejects an existing non-FK constraint or index with +``IllegalStateException`` instead of retaining a second hidden definition; +replace the definition explicitly with ``setConstraint()`` first. Reading, +clearing or removing absent FK properties does not create a definition. +Replacing or clearing ``setConstraint()`` also replaces or clears what the +legacy FK accessors expose. These accessors inspect stored definitions; use +``getOperation()``/``getConstraintKind()`` to identify the active action. + Migration from the former combined model: * Replace table-constraint reads from ``getIndexes()`` with diff --git a/src/test/java/net/sf/jsqlparser/statement/alter/AlterForeignKeyStateTest.java b/src/test/java/net/sf/jsqlparser/statement/alter/AlterForeignKeyStateTest.java new file mode 100644 index 000000000..75fe0a010 --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/alter/AlterForeignKeyStateTest.java @@ -0,0 +1,306 @@ +/*- + * #%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.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; +import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.schema.Database; +import net.sf.jsqlparser.schema.Table; +import net.sf.jsqlparser.statement.ReferentialAction; +import net.sf.jsqlparser.statement.ReferentialAction.Action; +import net.sf.jsqlparser.statement.ReferentialAction.Type; +import net.sf.jsqlparser.statement.create.table.ConstraintKind; +import net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint; +import net.sf.jsqlparser.statement.create.table.ForeignKeyReference; +import net.sf.jsqlparser.statement.create.table.Index; +import net.sf.jsqlparser.statement.create.table.KeyConstraint; +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.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +class AlterForeignKeyStateTest { + private static final String FK_SQL = "ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY " + + "(parent_id) REFERENCES app.parent(id) ON DELETE CASCADE"; + + @ParameterizedTest + @EnumSource(value = Dialect.class, names = {"MYSQL", "POSTGRESQL"}) + void legacyEditsUpdateParsedForeignKeysAndBothRenderers(Dialect dialect) throws Exception { + Alter statement = parse(FK_SQL, dialect); + AlterExpression action = first(statement); + ForeignKeyConstraint foreignKey = foreignKey(action); + assertEquals(List.of("parent_id"), action.getFkColumns()); + assertEquals("app", action.getFkSourceSchema()); + assertEquals("parent", action.getFkSourceTable()); + assertSame(foreignKey.getReferencedColumnNames(), action.getFkSourceColumns()); + assertSame(foreignKey.getReferentialAction(Type.DELETE), + action.getReferentialAction(Type.DELETE)); + + action.setFkColumns(List.of("owner_id")); + action.setFkSourceSchema("archive"); + action.setFkSourceTable("owners"); + action.setFkSourceColumns(new ArrayList<>(List.of("owner_key"))); + action.setReferentialAction(Type.DELETE, Action.RESTRICT); + + assertSame(foreignKey, action.getConstraint()); + assertEquals("fk", foreignKey.getName()); + assertRoundTrip("ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (owner_id) " + + "REFERENCES archive.owners(owner_key) ON DELETE RESTRICT", statement, dialect); + } + + @Test + void actionColumnListsAndAttributesSurviveLegacyAndStructuredMutations() throws Exception { + Alter statement = parse("ALTER TABLE posts ADD CONSTRAINT fk FOREIGN KEY " + + "(tenant_id, author_id) REFERENCES users(tenant_id, id) MATCH SIMPLE " + + "ON DELETE SET NULL (author_id) DEFERRABLE INITIALLY DEFERRED NOT VALID", + Dialect.POSTGRESQL); + AlterExpression action = first(statement); + ForeignKeyConstraint foreignKey = foreignKey(action); + ReferentialAction delete = foreignKey.getReferentialAction(Type.DELETE); + assertSame(delete, action.getReferentialAction(Type.DELETE)); + action.setOnDeleteSetNull(true); + assertSame(delete, action.getReferentialAction(Type.DELETE)); + assertEquals(List.of("author_id"), delete.getColumnNames()); + + action.getReferentialAction(Type.DELETE).setColumnNames(List.of("tenant_id")); + action.setReferentialAction(Type.DELETE, Action.SET_DEFAULT); + assertSame(delete, foreignKey.getReferentialAction(Type.DELETE)); + assertEquals(List.of("tenant_id"), delete.getColumnNames()); + assertRoundTrip("ALTER TABLE posts ADD CONSTRAINT fk FOREIGN KEY " + + "(tenant_id, author_id) REFERENCES users(tenant_id, id) MATCH SIMPLE " + + "ON DELETE SET DEFAULT (tenant_id) DEFERRABLE INITIALLY DEFERRED NOT VALID", + statement, Dialect.POSTGRESQL); + + delete.setAction(Action.CASCADE); + action.removeReferentialAction(Type.DELETE); + assertNull(foreignKey.getReferentialAction(Type.DELETE)); + assertFalse(action.isOnDeleteSetNull()); + assertRoundTrip("ALTER TABLE posts ADD CONSTRAINT fk FOREIGN KEY " + + "(tenant_id, author_id) REFERENCES users(tenant_id, id) MATCH SIMPLE " + + "DEFERRABLE INITIALLY DEFERRED NOT VALID", statement, Dialect.POSTGRESQL); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void builderSetterOrderAndFluentSchemaPreserveOneReference(boolean schemaFirst) + throws Exception { + AlterExpression action = new AlterExpression(); + action.setReferentialAction(Type.DELETE, Action.CASCADE); + if (schemaFirst) { + action.withFkSourceSchema("archive").withFkSourceTable("parents"); + } else { + action.withFkSourceTable("parents").withFkSourceSchema("archive"); + } + action.withFkColumns(List.of("parent_id")).withFkSourceColumns(List.of("id")); + assertNull(action.getOperation()); + action.setOperation(AlterOperation.ADD); + assertEquals("parents", action.getFkSourceTable()); + assertEquals("archive", action.getFkSourceSchema()); + assertEquals(ConstraintKind.FOREIGN_KEY, action.getConstraintKind()); + assertRoundTrip("ALTER TABLE child ADD FOREIGN KEY (parent_id) " + + "REFERENCES archive.parents(id) ON DELETE CASCADE", statement(action), + Dialect.POSTGRESQL); + } + + @Test + void keySnapshotsAndFluentAdditionsUseTheCurrentStructuredColumns() throws Exception { + Alter statement = parse(FK_SQL, Dialect.POSTGRESQL); + AlterExpression action = first(statement); + ForeignKeyConstraint foreignKey = foreignKey(action); + foreignKey.setColumnsNames(List.of("account_id")); + List snapshot = action.getFkColumns(); + assertEquals(List.of("account_id"), snapshot); + snapshot.set(0, "detached"); + assertEquals(List.of("account_id"), action.getFkColumns()); + action.addFkColumns("tenant_id").addFkColumns(List.of("region_id")); + action.addFkSourceColumns("tenant_id").addFkSourceColumns(List.of("region_id")); + action.getFkSourceColumns().set(0, "account_key"); + assertSame(foreignKey.getReferencedColumnNames(), action.getFkSourceColumns()); + assertEquals(List.of("account_id", "tenant_id", "region_id"), + foreignKey.getColumnsNames()); + assertRoundTrip("ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY " + + "(account_id, tenant_id, region_id) REFERENCES app.parent" + + "(account_key, tenant_id, region_id) ON DELETE CASCADE", statement, + Dialect.POSTGRESQL); + } + + @Test + void referenceReplacementAndNameEditsPreserveOtherQualifiedNameParts() throws Exception { + AlterExpression action = first(parse(FK_SQL, Dialect.POSTGRESQL)); + Table table = new Table("new_schema", "new_parent"); + table.setDatabase(new Database("catalog")); + ForeignKeyReference replacement = new ForeignKeyReference().withTable(table) + .withReferencedColumnNames(new ArrayList<>(List.of("new_id"))) + .withReferentialAction(Type.UPDATE, Action.NO_ACTION); + foreignKey(action).setReference(replacement); + assertEquals("new_schema", action.getFkSourceSchema()); + assertEquals("new_parent", action.getFkSourceTable()); + assertSame(replacement.getReferencedColumnNames(), action.getFkSourceColumns()); + assertNull(action.getReferentialAction(Type.DELETE)); + assertSame(replacement.getReferentialAction(Type.UPDATE), + action.getReferentialAction(Type.UPDATE)); + + action.setFkSourceSchema("\"changed.schema\""); + action.setFkSourceTable("\"changed.parent\""); + assertSame(table, replacement.getTable()); + assertEquals("catalog", table.getDatabase().getDatabaseName()); + assertEquals("\"changed.schema\"", table.getSchemaName()); + assertEquals("\"changed.parent\"", table.getName()); + } + + @Test + void replacingOrClearingTheConstraintDoesNotRestoreDetachedForeignKeyState() + throws Exception { + AlterExpression action = first(parse(FK_SQL, Dialect.POSTGRESQL)); + ForeignKeyConstraint old = foreignKey(action); + ForeignKeyConstraint replacement = new ForeignKeyConstraint() + .withColumnsNames(List.of("replacement_id")) + .withTable(new Table("replacement_parent")) + .withReferencedColumnNames(List.of("id")); + action.setConstraint(replacement); + assertEquals(List.of("replacement_id"), action.getFkColumns()); + assertEquals("replacement_parent", action.getFkSourceTable()); + assertNull(action.getFkSourceSchema()); + assertNull(action.getReferentialAction(Type.DELETE)); + assertEquals(Action.CASCADE, old.getReferentialAction(Type.DELETE).getAction()); + action.setConstraint(null); + assertNoForeignKeyValues(action); + action.setFkSourceTable("fresh_parent"); + assertNull(action.getFkColumns()); + assertNull(action.getFkSourceColumns()); + assertNull(action.getReferentialAction(Type.DELETE)); + assertEquals("fresh_parent", foreignKey(action).getTable().getName()); + } + + @Test + void absentReadsClearsAndRemovalsDoNotCreateAForeignKey() { + AlterExpression action = new AlterExpression(); + assertNoForeignKeyValues(action); + clearForeignKeyValues(action); + assertNull(action.getConstraint()); + assertNull(action.getIndex()); + assertNull(action.getOperation()); + } + + @Test + void storedForeignKeyDefinitionDoesNotChangeADropTarget() throws Exception { + Alter statement = parse("ALTER TABLE child DROP FOREIGN KEY fk", Dialect.MYSQL); + AlterExpression action = first(statement); + ForeignKeyConstraint stale = new ForeignKeyConstraint().withName("stale") + .withColumnsNames(List.of("unused")) + .withTable(new Table("unused_parent")) + .withReferencedColumnNames(List.of("unused_id")); + action.setConstraint(stale); + assertSame(stale, action.getConstraint()); + assertEquals(ConstraintKind.FOREIGN_KEY, action.getConstraintKind()); + assertRoundTrip("ALTER TABLE child DROP FOREIGN KEY fk", statement, Dialect.MYSQL); + } + + static Stream nonForeignKeyDefinitions() { + return Stream.of( + new AlterExpression().withOperation(AlterOperation.ADD).withIndex( + new Index().withType("INDEX").withName("ix") + .withColumnsNames(List.of("id"))), + new AlterExpression().withOperation(AlterOperation.ADD).withConstraint( + new KeyConstraint().withType("UNIQUE").withColumnsNames(List.of("id"))), + new AlterExpression().withOperation(AlterOperation.ADD) + .withPkColumns(List.of("id")), + new AlterExpression().withOperation(AlterOperation.ADD) + .withUkColumns(List.of("id")), + new AlterExpressionPrimaryKey().withPkColumns(List.of("id"))); + } + + @ParameterizedTest + @MethodSource("nonForeignKeyDefinitions") + void legacyForeignKeySettersDoNotOverwriteOtherDefinitions(AlterExpression action) { + Index originalIndex = action.getIndex(); + NamedConstraint originalConstraint = action.getConstraint(); + List originalPkColumns = action.getPkColumns(); + List originalUkColumns = action.getUkColumns(); + String before = action.toString(); + assertNoForeignKeyValues(action); + clearForeignKeyValues(action); + assertThrows(IllegalStateException.class, () -> action.setFkColumns(List.of("fk"))); + assertThrows(IllegalStateException.class, () -> action.setFkSourceSchema("schema")); + assertThrows(IllegalStateException.class, () -> action.setFkSourceTable("parent")); + assertThrows(IllegalStateException.class, () -> action.setFkSourceColumns(List.of("id"))); + assertThrows(IllegalStateException.class, + () -> action.setReferentialAction(Type.DELETE, Action.CASCADE)); + assertSame(originalIndex, action.getIndex()); + assertSame(originalConstraint, action.getConstraint()); + assertEquals(originalPkColumns, action.getPkColumns()); + assertEquals(originalUkColumns, action.getUkColumns()); + assertEquals(before, action.toString()); + } + + private static void assertNoForeignKeyValues(AlterExpression action) { + assertNull(action.getFkColumns()); + assertNull(action.getFkSourceSchema()); + assertNull(action.getFkSourceTable()); + assertNull(action.getFkSourceColumns()); + assertNull(action.getReferentialAction(Type.DELETE)); + assertFalse(action.isOnDeleteCascade()); + assertFalse(action.isOnDeleteRestrict()); + assertFalse(action.isOnDeleteSetNull()); + } + + private static void clearForeignKeyValues(AlterExpression action) { + action.setFkColumns(null); + action.setFkSourceSchema(null); + action.setFkSourceTable(null); + action.setFkSourceColumns(null); + action.removeReferentialAction(Type.DELETE); + action.setOnDeleteCascade(false); + action.setOnDeleteRestrict(false); + action.setOnDeleteSetNull(false); + } + + private static ForeignKeyConstraint foreignKey(AlterExpression action) { + return assertInstanceOf(ForeignKeyConstraint.class, action.getConstraint()); + } + + 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("child")); + statement.addAlterExpression(action); + return statement; + } + + private static Alter parse(String sql, Dialect dialect) throws JSQLParserException { + return (Alter) CCJSqlParserUtil.parse(sql, parser -> parser.withDialect(dialect)); + } + + private static void assertRoundTrip(String expected, Alter statement, Dialect dialect) + throws JSQLParserException { + assertEquals(expected, statement.toString()); + StringBuilder output = new StringBuilder(); + statement.accept(new StatementDeParser(output)); + assertEquals(expected, output.toString()); + assertEquals(expected, parse(output.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 4fbd3f445..3f8f98b10 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 @@ -20,12 +20,40 @@ import net.sf.jsqlparser.util.validation.metadata.Named; import net.sf.jsqlparser.util.validation.metadata.NamedObject; import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.parser.feature.FeatureConfiguration; +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.util.validation.ValidationTestAsserts; import net.sf.jsqlparser.util.validation.feature.DatabaseType; import org.junit.jupiter.api.Test; public class AlterValidatorTest extends ValidationTestAsserts { + @Test + void validatesForeignKeysFromTheirCurrentStructuredDefinition() throws JSQLParserException { + Alter alter = (Alter) CCJSqlParserUtil.parse( + "ALTER TABLE child ADD FOREIGN KEY (parent_id) REFERENCES old_parent (id)"); + ForeignKeyConstraint foreignKey = + (ForeignKeyConstraint) alter.getAlterExpressions().get(0).getConstraint(); + foreignKey.setTable(new Table("app", "new_parent")); + foreignKey.setColumnsNames(List.of("new_parent_id")); + foreignKey.setReferencedColumnNames(List.of("new_id")); + List names = new ArrayList<>(); + DatabaseMetaDataValidation metadata = name -> { + names.add(name); + return true; + }; + + assertTrue(Validation.validate(alter, Validation.createValidationContext( + new FeatureConfiguration(), List.of(metadata))).isEmpty()); + assertEquals(List.of(new Named(NamedObject.table, "child"), + new Named(NamedObject.table, "app.new_parent"), + new Named(NamedObject.column, "new_id"), + new Named(NamedObject.column, "new_parent_id")), names); + } + @Test void validatesRenamedConstraintTargetAsANewConstraint() { List names = new ArrayList<>(); From f34191ddce2e13f61dcf9a9c5247bb500f6673c8 Mon Sep 17 00:00:00 2001 From: minleejae Date: Sun, 11 Oct 2026 18:42:32 +0900 Subject: [PATCH 2/2] Use key accessors when checking competing foreign key definitions Signed-off-by: minleejae --- .../net/sf/jsqlparser/statement/alter/AlterExpression.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 9b8cb69d5..dde3499d8 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -390,7 +390,8 @@ private ForeignKeyConstraint getForeignKeyConstraint() { private ForeignKeyConstraint foreignKeyForUpdate(boolean create) { ForeignKeyConstraint foreignKey = getForeignKeyConstraint(); if (foreignKey == null && create) { - if (constraint != null || index != null || pkColumns != null || ukColumns != null) { + if (constraint != null || index != null || getPkColumns() != null + || getUkColumns() != null) { throw new IllegalStateException( "Foreign-key setters cannot replace an existing definition; use setConstraint"); }