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 6e22f6eb7..b928bbda0 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -505,20 +505,57 @@ public void setUsingIfExists(boolean usingIfExists) { this.usingIfExists = 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 index; use + * {@link #setPkColumns(List)} to replace its keys, or {@link Index#getColumns()} for structured + * edits. + */ public List getPkColumns() { - return pkColumns; + return hasKeyIndex(Index.Kind.PRIMARY_KEY) ? index.getColumnsNames() : pkColumns; } + /** + * Replaces a structured PRIMARY_KEY's elements using {@link Index#setColumnsNames(List)}. The + * strings are not parsed and existing element attributes are not retained, even when the + * rendered SQL is unchanged. Use {@link Index#setColumns(List)} to preserve structured + * elements. Without a matching index, stores the legacy list. + */ public void setPkColumns(List pkColumns) { - this.pkColumns = pkColumns; + if (hasKeyIndex(Index.Kind.PRIMARY_KEY)) { + index.setColumnsNames(pkColumns); + this.pkColumns = null; + } else { + this.pkColumns = 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 index; use {@link #setUkColumns(List)} + * to replace its keys, or {@link Index#getColumns()} for structured edits. + */ public List getUkColumns() { - return ukColumns; + return hasKeyIndex(Index.Kind.UNIQUE) ? index.getColumnsNames() : ukColumns; } + /** + * Replaces a structured UNIQUE's elements using {@link Index#setColumnsNames(List)}. The + * strings are not parsed and existing element attributes are not retained, even when the + * rendered SQL is unchanged. Use {@link Index#setColumns(List)} to preserve structured + * elements. Without a matching index, stores the legacy list. + */ public void setUkColumns(List ukColumns) { - this.ukColumns = ukColumns; + if (hasKeyIndex(Index.Kind.UNIQUE)) { + index.setColumnsNames(ukColumns); + this.ukColumns = null; + } else { + this.ukColumns = ukColumns; + } + } + + private boolean hasKeyIndex(Index.Kind kind) { + return index != null && index.getKind() == kind; } public String getUkName() { @@ -1357,24 +1394,28 @@ public AlterExpression withColumnOldName(String columnOldName) { return this; } + /** Adds strings using the replacement semantics of {@link #setPkColumns(List)}. */ public AlterExpression addPkColumns(String... pkColumns) { List collection = Optional.ofNullable(getPkColumns()).orElseGet(ArrayList::new); Collections.addAll(collection, pkColumns); return this.withPkColumns(collection); } + /** Adds strings using the replacement semantics of {@link #setPkColumns(List)}. */ public AlterExpression addPkColumns(Collection pkColumns) { List collection = Optional.ofNullable(getPkColumns()).orElseGet(ArrayList::new); collection.addAll(pkColumns); return this.withPkColumns(collection); } + /** Adds strings using the replacement semantics of {@link #setUkColumns(List)}. */ public AlterExpression addUkColumns(String... ukColumns) { List collection = Optional.ofNullable(getUkColumns()).orElseGet(ArrayList::new); Collections.addAll(collection, ukColumns); return this.withUkColumns(collection); } + /** Adds strings using the replacement semantics of {@link #setUkColumns(List)}. */ public AlterExpression addUkColumns(Collection ukColumns) { List collection = Optional.ofNullable(getUkColumns()).orElseGet(ArrayList::new); collection.addAll(ukColumns); diff --git a/src/main/java/net/sf/jsqlparser/statement/create/table/Index.java b/src/main/java/net/sf/jsqlparser/statement/create/table/Index.java index d66833d59..bb60df7a0 100644 --- a/src/main/java/net/sf/jsqlparser/statement/create/table/Index.java +++ b/src/main/java/net/sf/jsqlparser/statement/create/table/Index.java @@ -134,12 +134,22 @@ public void appendConstraintAttributesTo(StringBuilder sql) { } } + /** + * Returns a mutable snapshot of the rendered key elements, including their options. An index + * without columns returns an empty snapshot. Use {@link #getColumns()} for structured edits. + */ public List getColumnsNames() { - return columns.stream() - .map(ColumnParams::toString) - .collect(toList()); + return columns == null ? new ArrayList<>() + : columns.stream() + .map(ColumnParams::toString) + .collect(toList()); } + /** + * Replaces all key elements with plain {@link ColumnParams} wrapping the supplied strings, or + * clears them for null. The strings are not parsed and existing expressions and element options + * are not retained. Use {@link #setColumns(List)} to preserve structured elements. + */ public void setColumnsNames(List list) { if (list == null) { this.columns = Collections.emptyList(); diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index 92f99568d..de633d792 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -1562,13 +1562,10 @@ public class CCJSqlParser extends AbstractJSqlParser { return false; } - /** Keeps legacy ALTER key accessors populated from the common structured definition. */ + /** Attaches the structured index and populates the remaining legacy constraint metadata. */ private static void setAlterTableIndex(AlterExpression alterExp, Index index) { alterExp.setIndex(index); - if (index.getKind() == Index.Kind.PRIMARY_KEY) { - alterExp.setPkColumns(index.getColumnsNames()); - } else if (index.getKind() == Index.Kind.UNIQUE) { - alterExp.setUkColumns(index.getColumnsNames()); + if (index.getKind() == Index.Kind.UNIQUE) { alterExp.setUkName(index instanceof NamedConstraint ? ((NamedConstraint) index).getIndexName() : index.getName()); alterExp.setUk(index.getType().toUpperCase(Locale.ROOT).contains("KEY")); diff --git a/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java new file mode 100644 index 000000000..ddda3cc7e --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java @@ -0,0 +1,166 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2019 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.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.schema.Column; +import net.sf.jsqlparser.statement.create.table.Index; +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.ValueSource; + +class AlterKeyAccessorsTest { + private List columns(AlterExpression action, boolean primary) { + return primary ? action.getPkColumns() : action.getUkColumns(); + } + + private void setColumns(AlterExpression action, boolean primary, List columns) { + if (primary) { + action.setPkColumns(columns); + } else { + action.setUkColumns(columns); + } + } + + private void assertDeparsed(Alter statement, String expected) { + assertEquals(expected, statement.toString()); + StringBuilder output = new StringBuilder(); + statement.accept(new StatementDeParser(output), null); + assertEquals(expected, output.toString()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void readsCurrentIndexAndAppliesSettersAndFluentAdditions(boolean primary) throws Exception { + String prefix = "ALTER TABLE t ADD " + (primary ? "PRIMARY KEY" : "UNIQUE"); + Alter statement = (Alter) CCJSqlParserUtil.parse(prefix + " (a)"); + AlterExpression action = statement.getAlterExpressions().get(0); + Index originalIndex = action.getIndex(); + originalIndex.setColumnsNames(List.of("b")); + List snapshot = columns(action, primary); + assertEquals(List.of("b"), snapshot); + snapshot.set(0, "c"); + assertEquals(List.of("b"), columns(action, primary)); + assertDeparsed(statement, prefix + " (b)"); + setColumns(action, primary, List.of("c")); + assertDeparsed(statement, prefix + " (c)"); + if (primary) { + action.addPkColumns("d").addPkColumns(List.of("e")); + } else { + action.addUkColumns("d").addUkColumns(List.of("e")); + } + assertEquals(List.of("c", "d", "e"), columns(action, primary)); + assertSame(originalIndex, action.getIndex()); + assertDeparsed(statement, prefix + " (c, d, e)"); + setColumns(action, primary, null); + assertTrue(columns(action, primary).isEmpty()); + assertTrue(originalIndex.getColumns().isEmpty()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void replacingWithBareNamesDropsElementOptionsButKeepsIndexMetadata(boolean primary) + throws Exception { + String prefix = "ALTER TABLE t ADD CONSTRAINT key_t " + + (primary ? "PRIMARY KEY" : "UNIQUE"); + Alter statement = (Alter) CCJSqlParserUtil.parse(prefix + " (a DESC, b) DEFERRABLE"); + AlterExpression action = statement.getAlterExpressions().get(0); + Index index = action.getIndex(); + Index.ColumnParams first = index.getColumns().get(0); + assertEquals("a DESC", first.toString()); + setColumns(action, primary, List.of("b", "a")); + assertEquals(List.of("b", "a"), columns(action, primary)); + assertNull(index.getColumns().get(1).getSortOrder()); + assertNull(index.getColumns().get(1).getParams()); + assertNotSame(first, index.getColumns().get(1)); + assertSame(index, action.getIndex()); + assertEquals("key_t", index.getName()); + assertDeparsed(statement, prefix + " (b, a) DEFERRABLE"); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void settingAnUnchangedSnapshotPreservesSqlButRebuildsPlainElements(boolean primary) + throws Exception { + String prefix = "ALTER TABLE t ADD " + (primary ? "PRIMARY KEY" : "UNIQUE"); + Alter statement = (Alter) CCJSqlParserUtil.parse(prefix + " (a DESC, b)"); + AlterExpression action = statement.getAlterExpressions().get(0); + Index index = action.getIndex(); + Index.ColumnParams expression = new Index.ColumnParams(new Column("a")) + .withExpressionParenthesized(false) + .withSortOrder(Index.ColumnParams.SortOrder.DESC); + index.getColumns().set(0, expression); + setColumns(action, primary, new ArrayList<>(columns(action, primary))); + Index.ColumnParams replacement = index.getColumns().get(0); + assertNotSame(expression, replacement); + assertEquals("a DESC", replacement.getColumnName()); + assertNull(replacement.getExpression()); + assertNull(replacement.getSortOrder()); + assertDeparsed(statement, prefix + " (a DESC, b)"); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void structuredReorderingRetainsElementIdentityAndOptions(boolean primary) throws Exception { + String prefix = "ALTER TABLE t ADD " + (primary ? "PRIMARY KEY" : "UNIQUE"); + Alter statement = (Alter) CCJSqlParserUtil.parse(prefix + " (a DESC, b)"); + AlterExpression action = statement.getAlterExpressions().get(0); + Index index = action.getIndex(); + Index.ColumnParams first = index.getColumns().get(0); + Index.ColumnParams second = index.getColumns().get(1); + index.setColumns(List.of(second, first)); + assertSame(first, index.getColumns().get(1)); + assertSame(second, index.getColumns().get(0)); + assertEquals(Index.ColumnParams.SortOrder.DESC, first.getSortOrder()); + assertEquals(List.of("b", "a DESC"), columns(action, primary)); + assertDeparsed(statement, prefix + " (b, a DESC)"); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void supportsAnIndexWithoutColumnsAndFluentPopulation(boolean primary) { + Index index = new Index().withType(primary ? "PRIMARY KEY" : "UNIQUE"); + AlterExpression action = new AlterExpression().withOperation(AlterOperation.ADD) + .withIndex(index); + assertTrue(index.getColumnsNames().isEmpty()); + assertTrue(columns(action, primary).isEmpty()); + index.getColumnsNames().add("detached"); + assertNull(index.getColumns()); + if (primary) { + action.addPkColumns("a"); + } else { + action.addUkColumns("a"); + } + assertEquals(List.of("a"), columns(action, primary)); + index.setColumns(null); + assertTrue(columns(action, primary).isEmpty()); + setColumns(action, primary, List.of("b")); + assertEquals(List.of("b"), index.getColumnsNames()); + } + + @Test + void retainsLegacyOnlyConstructionAndIgnoresUnrelatedIndexes() { + AlterExpression action = new AlterExpression().withOperation(AlterOperation.ADD) + .withPkColumns(new ArrayList<>(List.of("a"))); + action.getPkColumns().add("b"); + assertEquals("ADD PRIMARY KEY (a, b)", action.toString()); + 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()); + assertEquals(List.of("other"), unrelated.getColumnsNames()); + } +}