Skip to content

Use one foreign-key state in ALTER TABLE #2764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
manticore-projects merged 2 commits into from
Oct 11, 2026
Merged

Use one foreign-key state in ALTER TABLE #2764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd8b5c3
Use one foreign-key state in ALTER TABLE
minleejae Oct 11, 2026
f34191d
Use key accessors when checking competing foreign key definitions
minleejae Oct 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 112 additions & 71 deletions src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -42,7 +41,6 @@ public enum TableRenameKeyword {
NONE, TO, AS
}

private final Set<ReferentialAction> referentialActions = new LinkedHashSet<>(2);
private AlterOperation operation;
private String optionalSpecifier;
private Table newTable;
Expand All @@ -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<String> 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<String> fkSourceColumns;
private boolean uk;
private boolean ukTypeSpecified;
private boolean useEqual;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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<String> 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<String> fkColumns) {
this.fkColumns = fkColumns;
ForeignKeyConstraint foreignKey = foreignKeyForUpdate(fkColumns != null);
if (foreignKey != null) {
if (fkColumns == null) {
foreignKey.setColumns(null);
} else {
foreignKey.setColumnsNames(fkColumns);
}
}
}

/**
Expand All @@ -367,16 +366,63 @@ public void setFkColumns(List<String> 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 || getPkColumns() != null
|| getUkColumns() != 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<String> parts = new ArrayList<>(table.getNameParts());
while (parts.size() <= partIndex) {
parts.add(null);
}
parts.set(partIndex, value);
table.setNameParts(parts);
}

public List<ColumnDataType> getColDataTypeList() {
Expand Down Expand Up @@ -456,21 +502,29 @@ public List<ColumnSetVisibility> 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<String> 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<String> fkSourceColumns) {
this.fkSourceColumns = fkSourceColumns;
ForeignKeyConstraint foreignKey = foreignKeyForUpdate(fkSourceColumns != null);
if (foreignKey != null) {
foreignKey.setReferencedColumnNames(fkSourceColumns);
}
}

public String getNewTableName() {
Expand Down Expand Up @@ -677,7 +731,7 @@ private ConstraintKind getAddedConstraintKind() {
if (ukColumns != null) {
return ConstraintKind.UNIQUE;
}
return fkColumns != null ? ConstraintKind.FOREIGN_KEY : ConstraintKind.OTHER;
return ConstraintKind.OTHER;
}

private boolean hasColumnOrNamedTarget() {
Expand Down Expand Up @@ -1451,7 +1505,7 @@ private void toStringGeneral(StringBuilder b, Consumer<ColumnDataType> 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) {
Expand All @@ -1471,21 +1525,6 @@ private void toStringGeneral(StringBuilder b, Consumer<ColumnDataType> 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()) {
Expand Down Expand Up @@ -1574,7 +1613,7 @@ public AlterExpression withFkColumns(List<String> fkColumns) {

@Deprecated
public AlterExpression withFkSourceSchema(String fkSourceSchema) {
this.setFkSourceTable(fkSourceSchema);
this.setFkSourceSchema(fkSourceSchema);
return this;
}

Expand Down Expand Up @@ -1643,13 +1682,15 @@ public AlterExpression addUkColumns(Collection<String> ukColumns) {
return this.withUkColumns(collection);
}

/** Adds strings using the replacement semantics of {@link #setFkColumns(List)}. */
@Deprecated
public AlterExpression addFkColumns(String... fkColumns) {
List<String> collection = Optional.ofNullable(getFkColumns()).orElseGet(ArrayList::new);
Collections.addAll(collection, fkColumns);
return this.withFkColumns(collection);
}

/** Adds strings using the replacement semantics of {@link #setFkColumns(List)}. */
@Deprecated
public AlterExpression addFkColumns(Collection<String> fkColumns) {
List<String> collection = Optional.ofNullable(getFkColumns()).orElseGet(ArrayList::new);
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/net/sf/jsqlparser/util/validation/validator/AlterValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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());
Expand Down
Loading
Loading