diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterConstraintAttributes.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterConstraintAttributes.java index 0b09255e9..f02ca447f 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterConstraintAttributes.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterConstraintAttributes.java @@ -44,12 +44,12 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } 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 046e6bb23..935b23abd 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -29,6 +29,7 @@ import net.sf.jsqlparser.statement.create.table.Index; import net.sf.jsqlparser.statement.create.table.NamedConstraint; import net.sf.jsqlparser.statement.create.table.KeyConstraint; +import net.sf.jsqlparser.statement.create.table.TableElement; import net.sf.jsqlparser.statement.create.table.IndexOptionSource; import net.sf.jsqlparser.statement.create.table.ConstraintKind; import net.sf.jsqlparser.statement.create.table.PartitionDefinition; @@ -822,8 +823,12 @@ private ConstraintKind getAddedConstraintKind() { } private boolean hasColumnOrNamedTarget() { + return oldIndex != null || hasGeneralBodyTarget(); + } + + private boolean hasGeneralBodyTarget() { return columnName != null || colDataTypeList != null || constraintName != null - || oldIndex != null || commentText != null || columnSetNotNullList != null + || commentText != null || columnSetNotNullList != null || columnDropNotNullList != null || columnDropDefaultList != null && !columnDropDefaultList.isEmpty(); } @@ -835,12 +840,30 @@ private boolean hasColumnOrNamedTarget() { */ public boolean hasActiveTableDefinition() { return operation == AlterOperation.ADD && (constraint != null || index != null) - && !(constraintType != null && constraintSymbol != null) - && !isRenameOperation() && commentText == null && columnName == null - && colDataTypeList == null && columnSetNotNullList == null - && columnDropNotNullList == null - && (columnDropDefaultList == null || columnDropDefaultList.isEmpty()) - && constraintName == null; + && usesGeneralBody() && !hasGeneralBodyTarget(); + } + + /** + * Returns the original definition contributing to the current body, or null when another + * payload takes precedence. A legacy constraint header renders only its key elements, not every + * option of the returned definition. Stored definitions remain available through + * {@link #getConstraint()} and {@link #getIndex()}. + */ + public TableElement getActiveTableDefinition() { + if (hasActiveConstraintHeader()) { + return operation == AlterOperation.ADD && constraint instanceof KeyConstraint + ? constraint + : null; + } + if (!usesGeneralBody() || hasGeneralBodyTarget()) { + return null; + } + return constraint != null ? constraint : index; + } + + /** Whether the selected body renders the legacy constraint header. */ + public boolean hasActiveConstraintHeader() { + return hasExplicitConstraintHeader(); } /** Sets an index definition or target; a non-null value replaces any constraint definition. */ @@ -1166,7 +1189,7 @@ public final String toString() { protected void appendBody(StringBuilder b) { if (operation == AlterOperation.UNSPECIFIC) { b.append(optionalSpecifier); - } else if (hasExplicitConstraintHeader()) { + } else if (hasActiveConstraintHeader()) { toStringConstraintAlter(b); } else if (usesColumnActionBody()) { toStringAlterColumn(b); @@ -1197,7 +1220,7 @@ private boolean hasAlterColumnValueActions() { * overriding {@link #appendBody(StringBuilder)} must reflect their dispatch here as well. */ protected boolean usesColumnActionBody() { - return hasAlterColumnValueActions() && !hasExplicitConstraintHeader(); + return hasAlterColumnValueActions() && !hasActiveConstraintHeader(); } /** @@ -1206,8 +1229,13 @@ protected boolean usesColumnActionBody() { * deparsing. Subclasses with their own body dispatch override this method accordingly. */ public boolean hasActiveColumnDefinitions() { - return hasColumnDefinitionPayload() && operation != AlterOperation.UNSPECIFIC - && !hasExplicitConstraintHeader() && !usesColumnActionBody() + return hasColumnDefinitionPayload() && usesGeneralBody(); + } + + /** Whether this action selects the general body, before choosing its active payload. */ + protected boolean usesGeneralBody() { + return operation != AlterOperation.UNSPECIFIC + && !hasActiveConstraintHeader() && !usesColumnActionBody() && !isSimpleKeywordOperation() && !isRenameOperation() && !isDropSpecialOperation() && operation != AlterOperation.CONVERT && operation != AlterOperation.COLLATE && !isPartitionOperation(); diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionCharset.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionCharset.java index 2d1f3741b..0de75572e 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionCharset.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionCharset.java @@ -31,12 +31,12 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionDrop.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionDrop.java index 1fbb5a95e..c5caba3be 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionDrop.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionDrop.java @@ -30,8 +30,13 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { - return hasColumnDefinitionPayload() && !isDropSpecialOperation() + public boolean hasActiveConstraintHeader() { + return false; + } + + @Override + protected boolean usesGeneralBody() { + return !isDropSpecialOperation() && getOperation() != AlterOperation.DROP_PARTITION; } diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionOrderBy.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionOrderBy.java index 98e795923..4d9177521 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionOrderBy.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionOrderBy.java @@ -41,12 +41,12 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPartition.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPartition.java index 8297531fc..f1ad1da99 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPartition.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPartition.java @@ -276,12 +276,12 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPrimaryKey.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPrimaryKey.java index b89fd40d3..fe9c5a0bb 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPrimaryKey.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionPrimaryKey.java @@ -45,15 +45,20 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } + @Override + public KeyConstraint getActiveTableDefinition() { + return getConstraint(); + } + @Override public KeyConstraint getConstraint() { return (KeyConstraint) super.getConstraint(); diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionRename.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionRename.java index e296ac92f..0b7d4309c 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionRename.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionRename.java @@ -56,8 +56,13 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { - return getOperation() == AlterOperation.RENAME && hasColumnDefinitionPayload(); + public boolean hasActiveConstraintHeader() { + return false; + } + + @Override + protected boolean usesGeneralBody() { + return getOperation() == AlterOperation.RENAME; } @Override diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionTableOption.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionTableOption.java index 5db554653..e717cf3be 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionTableOption.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpressionTableOption.java @@ -47,12 +47,12 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/AlterForeignDataOptions.java b/src/main/java/net/sf/jsqlparser/statement/alter/AlterForeignDataOptions.java index 03b79dc81..921410d7f 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterForeignDataOptions.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterForeignDataOptions.java @@ -53,12 +53,12 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } diff --git a/src/main/java/net/sf/jsqlparser/statement/alter/RelationAlterAction.java b/src/main/java/net/sf/jsqlparser/statement/alter/RelationAlterAction.java index 2de16add3..42a618008 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/RelationAlterAction.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/RelationAlterAction.java @@ -252,12 +252,12 @@ protected boolean usesColumnActionBody() { } @Override - public boolean hasActiveColumnDefinitions() { + public boolean hasActiveConstraintHeader() { return false; } @Override - public boolean hasActiveTableDefinition() { + protected boolean usesGeneralBody() { return false; } diff --git a/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java b/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java index 739232035..071b1c39f 100644 --- a/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java +++ b/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java @@ -33,6 +33,7 @@ import net.sf.jsqlparser.statement.create.table.KeyColumnSource; import net.sf.jsqlparser.statement.create.table.IndexOptionSource; import net.sf.jsqlparser.statement.create.table.NotNullConstraint; +import net.sf.jsqlparser.statement.create.table.NamedConstraint; import net.sf.jsqlparser.statement.create.table.TableElement; import net.sf.jsqlparser.statement.create.table.TablePartitioning; import net.sf.jsqlparser.statement.create.table.PartitionBound; @@ -52,68 +53,83 @@ public static void visit(CreateIndex createIndex, Consumer expressio accept(createIndex.getWhere(), expressions); } - /** Visits the structured definitions and expressions belonging to a single ALTER action. */ + /** + * Visits expressions and tables contributing to the rendered ALTER action. Inactive metadata is + * retained by the AST's getters but is not visited. + */ public static void visit(AlterExpression action, Consumer expressions, Consumer tables) { + if (visitSpecialBody(action, expressions, tables)) { + return; + } + if (action.getOperation() == AlterOperation.RENAME_TABLE + && action.getOldIndex() == null + && !(action instanceof net.sf.jsqlparser.statement.alter.AlterExpressionDrop)) { + accept(action.getNewTable(), tables); + } + action.getColumnDefaults() + .forEach(column -> accept(column.getDefaultExpression(), expressions)); + if (action.hasActiveColumnDefinitions()) { + action.getColDataTypeList().forEach(column -> visit(column, expressions, tables)); + } + TableElement definition = action.getActiveTableDefinition(); + if (action.hasActiveConstraintHeader() && definition instanceof KeyColumnSource) { + visitKeys((KeyColumnSource) definition, expressions); + } else if (definition != null) { + visit(definition, expressions, tables); + } + } + + private static boolean visitSpecialBody(AlterExpression action, + Consumer expressions, + Consumer
tables) { if (action instanceof net.sf.jsqlparser.statement.alter.AlterExpressionOrderBy) { ((net.sf.jsqlparser.statement.alter.AlterExpressionOrderBy) action).getOrderByElements() .forEach(element -> accept(element.getExpression(), expressions)); - } - if (action instanceof net.sf.jsqlparser.statement.alter.RelationAlterAction) { + } else if (action instanceof net.sf.jsqlparser.statement.alter.RelationAlterAction) { net.sf.jsqlparser.statement.alter.RelationAlterAction relation = (net.sf.jsqlparser.statement.alter.RelationAlterAction) action; relation.visitExpressions(expressions); relation.visitTables(tables); - } - if (action instanceof net.sf.jsqlparser.statement.alter.AlterExpressionTableOption) { + } else if (action instanceof net.sf.jsqlparser.statement.alter.AlterExpressionTableOption) { visit(((net.sf.jsqlparser.statement.alter.AlterExpressionTableOption) action) .getStructuredTableOption(), expressions, tables); - } - if (action.getOperation() == AlterOperation.RENAME_TABLE) { - accept(action.getNewTable(), tables); - } - if (action instanceof AlterForeignDataOptions) { + } else if (action instanceof AlterForeignDataOptions) { ForeignDataOption.visitExpressions(((AlterForeignDataOptions) action).getOptions(), expressions); - } - if (action.getColumnSetDefaultList() != null) { - action.getColumnSetDefaultList() - .forEach(column -> accept(column.getDefaultExpression(), expressions)); - } - if (action.getColDataTypeList() != null) { - action.getColDataTypeList().forEach(column -> visit(column, expressions, tables)); - } - if (action.getConstraint() != null) { - visit(action.getConstraint(), expressions, tables); - } - if (action.getIndex() != null) { - visit(action.getIndex(), expressions, tables); - } - if (action instanceof AlterExpressionPartition) { - AlterExpressionPartition partition = (AlterExpressionPartition) action; - switch (partition.getOperation()) { - case ATTACH_PARTITION: - accept(partition.getPartitionTable(), tables); - visit(partition.getPartitionBound(), expressions); - break; - case DETACH_PARTITION: - accept(partition.getPartitionTable(), tables); - break; - case EXCHANGE_PARTITION: - accept(partition.getExchangeTable(), tables); - break; - case PARTITION_BY: - visit(partition.getPartitioning(), expressions); - break; - default: - break; - } - } - if (action instanceof AlterExpressionPrimaryKey) { + } else if (action instanceof AlterExpressionPartition) { + visitPartitionAction((AlterExpressionPartition) action, expressions, tables); + } else if (action instanceof AlterExpressionPrimaryKey) { AlterExpressionPrimaryKey primaryKey = (AlterExpressionPrimaryKey) action; + visit(primaryKey.getConstraint(), expressions, tables); if (primaryKey.isUsingHash()) { accept(primaryKey.getBucketCount(), expressions); } + } else { + return action instanceof net.sf.jsqlparser.statement.alter.AlterExpressionCharset + || action instanceof net.sf.jsqlparser.statement.alter.AlterConstraintAttributes; + } + return true; + } + + private static void visitPartitionAction(AlterExpressionPartition partition, + Consumer expressions, Consumer
tables) { + switch (partition.getOperation()) { + case ATTACH_PARTITION: + accept(partition.getPartitionTable(), tables); + visit(partition.getPartitionBound(), expressions); + break; + case DETACH_PARTITION: + accept(partition.getPartitionTable(), tables); + break; + case EXCHANGE_PARTITION: + accept(partition.getExchangeTable(), tables); + break; + case PARTITION_BY: + visit(partition.getPartitioning(), expressions); + break; + default: + break; } } @@ -195,22 +211,22 @@ public static void visit(TableElement element, Consumer expressions, if (element instanceof LikeClause) { accept(((LikeClause) element).getTable(), tables); } else if (element instanceof ColumnDefinition) { - ColumnDefinition column = (ColumnDefinition) element; - if (column.getColumnOptions() != null) { - for (ColumnOption option : column.getColumnOptions()) { - option.visitExpressions(expressions); - if (option.getForeignKeyReference() != null) { - accept(option.getForeignKeyReference().getTable(), tables); - } - if (option.getConstraint() != null) { - visit(option.getConstraint(), expressions, tables); - } - } - } - if (column instanceof AlterExpression.ColumnDataType) { - accept(((AlterExpression.ColumnDataType) column).getUsingExpression(), expressions); - } - } else if (element instanceof NotNullConstraint) { + visitColumn((ColumnDefinition) element, expressions, tables); + } else if (element instanceof NamedConstraint) { + visitConstraint((NamedConstraint) element, expressions, tables); + } + if (element instanceof KeyColumnSource) { + visitKeys((KeyColumnSource) element, expressions); + } + if (element instanceof IndexOptionSource) { + visitOptions(((IndexOptionSource) element).getIndexOptions().getStorageParameters(), + expressions); + } + } + + private static void visitConstraint(NamedConstraint element, Consumer expressions, + Consumer
tables) { + if (element instanceof NotNullConstraint) { accept(((NotNullConstraint) element).getColumn(), expressions); } else if (element instanceof CheckConstraint) { accept(((CheckConstraint) element).getExpression(), expressions); @@ -223,18 +239,42 @@ public static void visit(TableElement element, Consumer expressions, } else if (element instanceof ForeignKeyConstraint) { accept(((ForeignKeyConstraint) element).getTable(), tables); } - if (element instanceof KeyColumnSource) { - List keys = ((KeyColumnSource) element).getColumns(); - if (keys != null) { - for (KeyElement key : keys) { - accept(key.getExpression(), expressions); - visitOptions(key.getOperatorClassParameters(), expressions); - } + } + + private static void visitColumn(ColumnDefinition column, Consumer expressions, + Consumer
tables) { + if (column instanceof AlterExpression.ColumnDataType + && ((AlterExpression.ColumnDataType) column).getIdentityAlterations() != null) { + return; + } + if (column.getColumnOptions() != null) { + for (ColumnOption option : column.getColumnOptions()) { + visitColumnOption(option, expressions, tables); } } - if (element instanceof IndexOptionSource) { - visitOptions(((IndexOptionSource) element).getIndexOptions().getStorageParameters(), - expressions); + if (column instanceof AlterExpression.ColumnDataType) { + accept(((AlterExpression.ColumnDataType) column).getUsingExpression(), expressions); + } + } + + private static void visitColumnOption(ColumnOption option, Consumer expressions, + Consumer
tables) { + option.visitExpressions(expressions); + if (option.getKind() == ColumnOption.Kind.REFERENCE + && option.getForeignKeyReference() != null) { + accept(option.getForeignKeyReference().getTable(), tables); + } + if (option.getKind() == ColumnOption.Kind.CONSTRAINT && option.getConstraint() != null) { + visit(option.getConstraint(), expressions, tables); + } + } + + private static void visitKeys(KeyColumnSource source, Consumer expressions) { + if (source.getColumns() != null) { + for (KeyElement key : source.getColumns()) { + accept(key.getExpression(), expressions); + visitOptions(key.getOperatorClassParameters(), expressions); + } } } 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 f52f9cd85..5f6dcbc70 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 @@ -27,6 +27,7 @@ import net.sf.jsqlparser.statement.create.table.ConstraintKind; import net.sf.jsqlparser.statement.create.table.KeyConstraint; import net.sf.jsqlparser.statement.create.table.Index; +import net.sf.jsqlparser.statement.create.table.TableElement; import net.sf.jsqlparser.util.TableDefinitionTraversal; import net.sf.jsqlparser.util.validation.ValidationCapability; import net.sf.jsqlparser.util.validation.ValidationUtil; @@ -49,6 +50,7 @@ public void validate(Alter alter) { public void validate(Alter alter, AlterExpression e) { TableDefinitionTraversal.visit(e, this::validateOptionalExpression, this::validateOptionalFromItem); + TableElement definition = e.getActiveTableDefinition(); for (ValidationCapability c : getCapabilities()) { validateOptionalColumnName(c, e.getColumnOldName()); @@ -64,7 +66,7 @@ public void validate(Alter alter, AlterExpression e) { ColumnSetNotNull::getColumnName)); } - if (e.getColDataTypeList() != null) { + if (e.hasActiveColumnDefinitions()) { boolean validateForExist = !EnumSet.of(AlterOperation.ADD).contains(e.getOperation()); validateOptionalColumnNames(c, @@ -80,15 +82,16 @@ public void validate(Alter alter, AlterExpression e) { validateOptionalColumnNames(c, e.getDropColumns()); } - validateConstraint(c, e.getConstraint()); - validateIndex(c, e); + validateConstraint(c, definition); + validateIndex(c, e, definition); } } - private void validateConstraint(ValidationCapability capability, NamedConstraint constraint) { - if (constraint == null) { + private void validateConstraint(ValidationCapability capability, TableElement definition) { + if (!(definition instanceof NamedConstraint)) { return; } + NamedConstraint constraint = (NamedConstraint) definition; validateOptionalName(capability, NamedObject.constraint, constraint.getName(), null, false, NamedObject.table); if (constraint instanceof ForeignKeyConstraint) { @@ -129,19 +132,20 @@ private boolean hasActiveDropColumns(AlterExpression expression) { } } - private void validateIndex(ValidationCapability capability, AlterExpression expression) { + private void validateIndex(ValidationCapability capability, AlterExpression expression, + TableElement definition) { Index index = expression.getIndex(); - if (index == null) { - return; - } boolean rename = expression.getOperation() == AlterOperation.RENAME_INDEX || expression.getOperation() == AlterOperation.RENAME_KEY; + if (index == null || definition != index && !rename) { + return; + } if (rename && expression.getOldIndex() != null) { validateName(capability, NamedObject.index, expression.getOldIndex().getName()); } boolean introducedName = expression.getOperation() == AlterOperation.ADD || rename; validateOptionalName(capability, NamedObject.index, index.getName(), null, !introducedName); - if (index.getColumns() != null) { + if (definition == index && index.getColumns() != null) { validateOptionalColumnNames(capability, index.getColumns().stream() .filter(key -> !key.isExpression()).map(KeyElement::getColumnName) .collect(toList()), NamedObject.index); diff --git a/src/site/sphinx/usage.rst b/src/site/sphinx/usage.rst index 901608d62..d6f197648 100644 --- a/src/site/sphinx/usage.rst +++ b/src/site/sphinx/usage.rst @@ -689,9 +689,24 @@ its column definitions through the general ALTER body. ``getColDataTypeList()`` continues to expose stored metadata even when another body takes precedence. ``AlterDeParser`` uses the active path, so expression visitors rewrite the selected default or column expressions without rendering inactive metadata. +``TableDefinitionTraversal`` and the table-name finder visit the expressions +and tables belonging to the selected body. The metadata validator uses that +same selection. For example, adding a column definition to an action that still +stores an earlier foreign key renders the column and does not require the +inactive foreign table or its columns to exist. ``getConstraint()`` and +``getIndex()`` still expose the stored objects for direct inspection. + +``getActiveTableDefinition()`` returns the original definition contributing to +the current body, or ``null`` when another payload takes precedence. A legacy +``ADD CONSTRAINT`` header contributes only its keys; its unused storage options +are not traversed. Column options likewise visit only the payload selected by +their ``Kind``, including after changing an option to ``FOREIGN_OPTIONS``. + Custom ``AlterExpression`` subclasses overriding ``appendBody`` should also -override ``usesColumnActionBody()`` and ``hasActiveColumnDefinitions()`` when -they change those dispatch rules. +reflect their dispatch in ``usesColumnActionBody()``, ``usesGeneralBody()`` and +``hasActiveConstraintHeader()``. Subclasses rendering a separate definition can +override ``getActiveTableDefinition()``. The general-body hook also determines +``hasActiveColumnDefinitions()``. Structured column attributes ============================ diff --git a/src/test/java/net/sf/jsqlparser/util/ActiveTableDefinitionTraversalTest.java b/src/test/java/net/sf/jsqlparser/util/ActiveTableDefinitionTraversalTest.java new file mode 100644 index 000000000..58e982c23 --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/util/ActiveTableDefinitionTraversalTest.java @@ -0,0 +1,251 @@ +/*- + * #%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.util; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import net.sf.jsqlparser.expression.LongValue; +import net.sf.jsqlparser.expression.StringValue; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.schema.Table; +import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect; +import net.sf.jsqlparser.statement.Statement; +import net.sf.jsqlparser.statement.alter.Alter; +import net.sf.jsqlparser.statement.alter.AlterExpression; +import net.sf.jsqlparser.statement.alter.AlterOperation; +import net.sf.jsqlparser.statement.alter.AlterExpression.ColumnDataType; +import net.sf.jsqlparser.statement.alter.AlterExpression.ColumnSetDefault; +import net.sf.jsqlparser.statement.create.table.ColDataType; +import net.sf.jsqlparser.statement.create.table.ColumnDefinition; +import net.sf.jsqlparser.statement.create.table.ColumnOption; +import net.sf.jsqlparser.statement.create.table.CreateTable; +import net.sf.jsqlparser.statement.create.table.ForeignDataOption; +import net.sf.jsqlparser.statement.create.table.IndexOption; +import net.sf.jsqlparser.statement.create.table.KeyConstraint; +import net.sf.jsqlparser.statement.create.table.KeyElement; +import net.sf.jsqlparser.statement.create.table.TableElement; +import net.sf.jsqlparser.util.deparser.StatementDeParser; +import net.sf.jsqlparser.util.validation.ValidationContext; +import net.sf.jsqlparser.util.validation.metadata.DatabaseMetaDataValidation; +import net.sf.jsqlparser.util.validation.validator.AlterValidator; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +class ActiveTableDefinitionTraversalTest { + @ParameterizedTest + @ValueSource(strings = {"FOREIGN KEY (parent_id) REFERENCES old_parent (id)", + "INDEX old_index (old_id)", "CONSTRAINT hidden_check CHECK (old_id > 0)"}) + void addedColumnHidesStoredDefinitionFromAllConsumers(String definition) throws Exception { + Alter alter = parse("ALTER TABLE child ADD " + definition); + AlterExpression action = first(alter); + TableElement original = action.getActiveTableDefinition(); + action.addColDataType("added", new ColDataType("INT")); + assertEquals("ALTER TABLE child ADD added INT", alter.toString()); + assertNull(action.getActiveTableDefinition()); + assertTrue(original == action.getConstraint() || original == action.getIndex()); + assertEquals(Set.of("child"), new TablesNamesFinder<>().getTables(alter)); + assertTrue(expressions(action).isEmpty()); + assertEquals(List.of("child", "added"), validatedNames(alter, Set.of("child"))); + assertRoundTrip(alter, null); + } + + @Test + void operationChangesKeepForeignKeyStoredButDoNotVisitIt() throws Exception { + Alter alter = parse("ALTER TABLE child ADD FOREIGN KEY (parent_id) REFERENCES parent (id)"); + AlterExpression action = first(alter); + Object original = action.getConstraint(); + action.setOperation(AlterOperation.DROP_FOREIGN_KEY); + action.setConstraintName("fk"); + assertSame(original, action.getConstraint()); + assertNull(action.getActiveTableDefinition()); + assertEquals(Set.of("child"), new TablesNamesFinder<>().getTables(alter)); + assertEquals(List.of("child", "fk"), validatedNames(alter, Set.of("child", "fk"))); + assertRoundTrip(alter, Dialect.MYSQL); + } + + @Test + void activeForeignKeyStillVisitsItsTableAndBothColumnLists() throws Exception { + Alter alter = parse("ALTER TABLE child ADD FOREIGN KEY (parent_id) REFERENCES parent (id)"); + assertEquals(Set.of("child", "parent"), new TablesNamesFinder<>().getTables(alter)); + assertEquals(List.of("child", "parent", "id", "parent_id"), + validatedNames(alter, Set.of("child", "parent", "id", "parent_id"))); + assertRoundTrip(alter, null); + } + + @Test + void storedTableRenameDestinationIsInactiveWhenIndexNamesTakePrecedence() throws Exception { + Alter alter = parse("ALTER TABLE child RENAME INDEX original TO replacement"); + AlterExpression action = first(alter); + action.setOperation(AlterOperation.RENAME_TABLE); + action.setNewTable(new Table("hidden_table")); + assertFalse(alter.toString().contains("hidden_table")); + assertEquals(Set.of("child"), new TablesNamesFinder<>().getTables(alter)); + assertEquals("hidden_table", action.getNewTable().getName()); + } + + @Test + void defaultTraversalUsesTheSameBodyAsTheRenderer() throws Exception { + Alter alter = parse("ALTER TABLE child ALTER COLUMN id SET DEFAULT 1"); + AlterExpression action = first(alter); + ColumnDataType inactive = new ColumnDataType("hidden", false, new ColDataType("INT"), null); + inactive.addColumnOptions(ColumnOption.defaultValue(new LongValue(2))); + action.addColDataType(inactive); + assertEquals(List.of("1"), expressions(action)); + assertFalse(action.hasActiveColumnDefinitions()); + assertRoundTrip(alter, null); + } + + @Test + void dropSubclassCanSelectTheGeneralBodyAfterMutation() throws Exception { + Alter alter = parse("ALTER TABLE child DROP INDEX old_index"); + AlterExpression action = first(alter); + action.setOperation(AlterOperation.ADD); + action.setConstraint(first(parse("ALTER TABLE child ADD FOREIGN KEY (parent_id) " + + "REFERENCES parent (id)")).getConstraint()); + action.addColSetDefault(ColumnSetDefault.fromExpression("hidden", new LongValue(99))); + assertSame(action.getConstraint(), action.getActiveTableDefinition()); + assertEquals(Set.of("child", "parent"), new TablesNamesFinder<>().getTables(alter)); + assertTrue(expressions(action).isEmpty()); + assertEquals(List.of("child", "parent", "id", "parent_id"), + validatedNames(alter, Set.of("child", "parent", "id", "parent_id"))); + assertRoundTrip(alter, null); + } + + @ParameterizedTest + @CsvSource(delimiter = '|', value = { + "ALTER FOREIGN TABLE child OPTIONS (ADD optname 'active')|POSTGRESQL", + "ALTER TABLE child OWNER TO new_owner|POSTGRESQL", + "ALTER TABLE child ORDER BY active_id|MYSQL"}) + void fixedBodiesIgnoreInheritedDefinitionsAndDefaults(String sql, Dialect dialect) + throws Exception { + Alter alter = (Alter) CCJSqlParserUtil.parse(sql, parser -> parser.withDialect(dialect)); + AlterExpression action = first(alter); + List expectedExpressions = expressions(action); + action.setConstraint(first(parse("ALTER TABLE child ADD FOREIGN KEY (hidden_id) " + + "REFERENCES hidden_parent (id)")).getConstraint()); + action.addColDataType("hidden", new ColDataType("INT")); + action.addColSetDefault(ColumnSetDefault.fromExpression("hidden", new LongValue(99))); + action.setConstraintType("UNIQUE"); + action.setConstraintSymbol("hidden_key"); + action.setOperation(AlterOperation.RENAME_TABLE); + action.setNewTable(new Table("hidden_table")); + assertNull(action.getActiveTableDefinition()); + assertEquals(expectedExpressions, expressions(action)); + assertEquals(Set.of("child"), new TablesNamesFinder<>().getTables(alter)); + assertTrue(validatedNames(alter, Set.of("child", "active_id")).stream() + .noneMatch(name -> name.startsWith("hidden"))); + assertRoundTrip(alter, dialect); + } + + @Test + void identityAlterationDoesNotVisitDormantColumnOptionsOrUsingExpression() throws Exception { + Alter alter = (Alter) CCJSqlParserUtil.parse("ALTER TABLE child ALTER COLUMN id RESTART", + parser -> parser.withDialect(Dialect.POSTGRESQL)); + ColumnDataType column = first(alter).getColDataTypeList().get(0); + column.addColumnOptions(ColumnOption.defaultValue(new LongValue(99))); + column.setUsingExpression(new LongValue(100)); + assertEquals("100", column.getUsingExpression().toString()); + assertTrue(expressions(first(alter)).isEmpty()); + assertRoundTrip(alter, Dialect.POSTGRESQL); + } + + @Test + void legacyConstraintHeaderVisitsKeysWithoutDormantStorageOptions() throws Exception { + Alter alter = parse("ALTER TABLE child ADD CONSTRAINT UNIQUE KEY uq (id)"); + AlterExpression action = first(alter); + KeyConstraint key = (KeyConstraint) action.getConstraint(); + key.setColumns(List.of(new KeyElement(new LongValue(7)))); + key.setStorageParameters(List.of(new IndexOption("hidden", new LongValue(99), true))); + assertTrue(action.hasActiveConstraintHeader()); + assertSame(key, action.getActiveTableDefinition()); + assertEquals(List.of("7"), expressions(action)); + assertFalse(alter.toString().contains("99")); + } + + @Test + void specializedPrimaryKeyKeepsActiveKeysAndHashOptions() throws Exception { + Alter alter = (Alter) CCJSqlParserUtil.parse( + "ALTER TABLE child ALTER PRIMARY KEY USING COLUMNS (id) " + + "USING HASH WITH BUCKET_COUNT = 8 WITH (fillfactor = 70)", + parser -> parser.withDialect(Dialect.COCKROACHDB)); + assertEquals(List.of("70", "8"), expressions(first(alter))); + assertSame(first(alter).getConstraint(), first(alter).getActiveTableDefinition()); + assertRoundTrip(alter, Dialect.COCKROACHDB); + } + + @ParameterizedTest + @ValueSource(strings = {"CHECK (hidden_id > 0)", "REFERENCES hidden_parent (id)"}) + void columnOptionKindChangesHidePriorConstraintPayload(String optionSql) throws Exception { + CreateTable table = (CreateTable) CCJSqlParserUtil.parse( + "CREATE FOREIGN TABLE child (id INT " + optionSql + ") SERVER srv"); + ColumnDefinition column = table.getColumnDefinitions().get(0); + ColumnOption option = column.getColumnOptions().get(0); + Object stored = option.getConstraint() != null ? option.getConstraint() + : option.getForeignKeyReference(); + option.setForeignOptions( + List.of(new ForeignDataOption("column_name", new StringValue("active")))); + assertTrue(stored == option.getConstraint() || stored == option.getForeignKeyReference()); + List actual = new ArrayList<>(); + TableDefinitionTraversal.visit(table, expression -> actual.add(expression.toString()), + source -> fail("Inactive reference visited: " + source)); + assertEquals(List.of("'active'"), actual); + assertEquals(Set.of("child"), new TablesNamesFinder<>().getTables(table)); + assertRoundTrip(table, Dialect.POSTGRESQL); + } + + private static Alter parse(String sql) throws Exception { + return (Alter) CCJSqlParserUtil.parse(sql); + } + + private static AlterExpression first(Alter alter) { + return alter.getAlterExpressions().get(0); + } + + private static List expressions(AlterExpression action) { + List result = new ArrayList<>(); + TableDefinitionTraversal.visit(action, expression -> result.add(expression.toString()), + table -> { + }); + return result; + } + + private static List validatedNames(Alter alter, Set existing) { + List names = new ArrayList<>(); + DatabaseMetaDataValidation metadata = name -> { + names.add(name.getFqn()); + return existing.contains(name.getFqn()); + }; + AlterValidator validator = new AlterValidator(); + validator.setContext(new ValidationContext().setCapabilities(List.of(metadata))); + validator.validate(alter); + assertTrue(validator.getValidationErrors().isEmpty(), + validator.getValidationErrors().toString()); + return names; + } + + private static void assertRoundTrip(Statement statement, Dialect dialect) throws Exception { + StringBuilder sql = new StringBuilder(); + statement.accept(new StatementDeParser(sql)); + assertEquals(statement.toString(), sql.toString()); + Statement reparsed = CCJSqlParserUtil.parse(sql.toString(), parser -> { + if (dialect != null) { + parser.withDialect(dialect); + } + }); + assertEquals(sql.toString(), reparsed.toString()); + assertEquals(new TablesNamesFinder<>().getTables(reparsed), + new TablesNamesFinder<>().getTables(statement)); + } +}