From 84e7f9fa2a124b112f87e6235c5bb305c18407e0 Mon Sep 17 00:00:00 2001 From: minleejae Date: Sat, 12 Sep 2026 21:57:05 +0900 Subject: [PATCH 1/4] Keep ALTER key accessors connected to structured index state --- .../statement/alter/AlterExpression.java | 87 ++++++++++++++++- .../alter/AlterKeyAccessorsTest.java | 96 +++++++++++++++++++ 2 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.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 6e22f6eb7..db96b3967 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -11,6 +11,7 @@ import java.io.Serializable; import java.util.ArrayList; +import java.util.AbstractList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -505,20 +506,98 @@ public void setUsingIfExists(boolean usingIfExists) { this.usingIfExists = usingIfExists; } + /** Returns a live view when this action has a structured PRIMARY_KEY definition. */ public List getPkColumns() { - return pkColumns; + return hasKeyIndex(Index.Kind.PRIMARY_KEY) ? new KeyColumnNames(index) : pkColumns; } public void setPkColumns(List pkColumns) { - this.pkColumns = pkColumns; + if (hasKeyIndex(Index.Kind.PRIMARY_KEY)) { + replaceKeyColumns(pkColumns); + this.pkColumns = null; + } else { + this.pkColumns = pkColumns; + } } + /** Returns a live view when this action has a structured UNIQUE definition. */ public List getUkColumns() { - return ukColumns; + return hasKeyIndex(Index.Kind.UNIQUE) ? new KeyColumnNames(index) : ukColumns; } public void setUkColumns(List ukColumns) { - this.ukColumns = ukColumns; + if (hasKeyIndex(Index.Kind.UNIQUE)) { + replaceKeyColumns(ukColumns); + this.ukColumns = null; + } else { + this.ukColumns = ukColumns; + } + } + + private boolean hasKeyIndex(Index.Kind kind) { + return index != null && index.getKind() == kind; + } + + private void replaceKeyColumns(List names) { + List replacement = new ArrayList<>(); + if (names != null) { + List previous = index.getColumns(); + for (int i = 0; i < names.size(); i++) { + String name = names.get(i); + replacement.add(previous != null && i < previous.size() + && previous.get(i).toString().equals(name) ? previous.get(i) + : new Index.ColumnParams(name)); + } + } + index.setColumns(replacement); + } + + /** Adapts the legacy mutable name list without copying structured expressions to strings. */ + private static class KeyColumnNames extends AbstractList { + private final Index index; + + KeyColumnNames(Index index) { + this.index = index; + } + + @Override + public String get(int position) { + return index.getColumns().get(position).toString(); + } + + @Override + public int size() { + return index.getColumns() == null ? 0 : index.getColumns().size(); + } + + @Override + public String set(int position, String name) { + String previous = get(position); + if (!previous.equals(name)) { + List columns = new ArrayList<>(index.getColumns()); + columns.set(position, new Index.ColumnParams(name)); + index.setColumns(columns); + } + return previous; + } + + @Override + public void add(int position, String name) { + List columns = index.getColumns() == null ? new ArrayList<>() + : new ArrayList<>(index.getColumns()); + columns.add(position, new Index.ColumnParams(name)); + index.setColumns(columns); + modCount++; + } + + @Override + public String remove(int position) { + List columns = new ArrayList<>(index.getColumns()); + String previous = columns.remove(position).toString(); + index.setColumns(columns); + modCount++; + return previous; + } } public String getUkName() { 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..e140bd95a --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java @@ -0,0 +1,96 @@ +/*- + * #%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.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(); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void keepsGettersSettersAndMutableListsConnectedToTheIndex(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(); + action.getIndex().setColumnsNames(List.of("b")); + assertEquals(List.of("b"), columns(action, primary)); + if (primary) { + action.setPkColumns(List.of("c")); + } else { + action.setUkColumns(List.of("c")); + } + assertEquals(prefix + " (c)", statement.toString()); + columns(action, primary).add("d"); + columns(action, primary).set(0, "e"); + assertEquals(List.of("e", "d"), action.getIndex().getColumnsNames()); + assertEquals("e", columns(action, primary).remove(0)); + if (primary) { + action.addPkColumns("f"); + } else { + action.addUkColumns("f"); + } + assertEquals(prefix + " (d, f)", statement.toString()); + assertSame(originalIndex, action.getIndex()); + StringBuilder output = new StringBuilder(); + statement.accept(new StatementDeParser(output), null); + assertEquals(statement.toString(), output.toString()); + columns(action, primary).clear(); + assertTrue(action.getIndex().getColumns().isEmpty()); + } + + @Test + void preservesStructuredElementsAndIndexMetadataForUnchangedKeys() throws Exception { + Alter statement = (Alter) CCJSqlParserUtil.parse( + "ALTER TABLE t ADD UNIQUE (a, b) DEFERRABLE", + parser -> parser.withDialect( + net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect.POSTGRESQL)); + AlterExpression action = statement.getAlterExpressions().get(0); + Index index = action.getIndex(); + index.setName("uq"); + Index.ColumnParams expression = + new Index.ColumnParams(new net.sf.jsqlparser.schema.Column("a")) + .withExpressionParenthesized(false); + index.getColumns().set(0, expression); + String original = statement.toString(); + action.setUkColumns(new ArrayList<>(action.getUkColumns())); + assertSame(expression, index.getColumns().get(0)); + assertEquals(original, statement.toString()); + action.addUkColumns("c"); + assertSame(expression, index.getColumns().get(0)); + assertEquals("uq", index.getName()); + assertEquals(original.replace("(a, b)", "(a, b, c)"), statement.toString()); + assertTrue(statement.toString().endsWith("DEFERRABLE")); + assertEquals(List.of("a", "b", "c"), action.getUkColumns()); + } + + @Test + void retainsLegacyOnlyConstructionAndAllowsClearingStructuredKeys() { + AlterExpression action = new AlterExpression().withOperation(AlterOperation.ADD) + .withPkColumns(new ArrayList<>(List.of("a"))); + action.addPkColumns("b"); + assertEquals("ADD PRIMARY KEY (a, b)", action.toString()); + action.setIndex(new Index().withType("PRIMARY KEY").withColumnsNames(List.of("c"))); + assertEquals(List.of("c"), action.getPkColumns()); + action.setPkColumns(null); + assertTrue(action.getPkColumns().isEmpty()); + } +} From ee9ab98f0e0de4f224ebbe7018f6ae7bd0b31b77 Mon Sep 17 00:00:00 2001 From: minleejae Date: Sun, 13 Sep 2026 01:53:54 +0900 Subject: [PATCH 2/4] Clarify ALTER key element type for static analysis --- .../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 db96b3967..eab424af5 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -562,7 +562,8 @@ private static class KeyColumnNames extends AbstractList { @Override public String get(int position) { - return index.getColumns().get(position).toString(); + Index.ColumnParams column = index.getColumns().get(position); + return column.toString(); } @Override From b289ef6b4e759577a46a12a2149e807e44a08200 Mon Sep 17 00:00:00 2001 From: minleejae Date: Sun, 13 Sep 2026 10:34:03 +0900 Subject: [PATCH 3/4] Make remaining ALTER key element types explicit --- .../statement/alter/AlterExpression.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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 eab424af5..c03af8bda 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -544,9 +544,14 @@ private void replaceKeyColumns(List names) { List previous = index.getColumns(); for (int i = 0; i < names.size(); i++) { String name = names.get(i); - replacement.add(previous != null && i < previous.size() - && previous.get(i).toString().equals(name) ? previous.get(i) - : new Index.ColumnParams(name)); + if (previous != null && i < previous.size()) { + Index.ColumnParams previousColumn = previous.get(i); + if (previousColumn.toString().equals(name)) { + replacement.add(previousColumn); + continue; + } + } + replacement.add(new Index.ColumnParams(name)); } } index.setColumns(replacement); @@ -594,7 +599,8 @@ public void add(int position, String name) { @Override public String remove(int position) { List columns = new ArrayList<>(index.getColumns()); - String previous = columns.remove(position).toString(); + Index.ColumnParams removedColumn = columns.remove(position); + String previous = removedColumn.toString(); index.setColumns(columns); modCount++; return previous; From cc6e528501de7d45c3e075bcdf4268a71345bd60 Mon Sep 17 00:00:00 2001 From: minleejae Date: Sun, 13 Sep 2026 18:00:45 +0900 Subject: [PATCH 4/4] Simplify ALTER key adapters and document replacement semantics --- .../statement/alter/AlterExpression.java | 107 ++++-------- .../statement/create/table/Index.java | 16 +- .../net/sf/jsqlparser/parser/JSqlParserCC.jjt | 7 +- .../alter/AlterKeyAccessorsTest.java | 162 +++++++++++++----- 4 files changed, 162 insertions(+), 130 deletions(-) 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 c03af8bda..b928bbda0 100644 --- a/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java +++ b/src/main/java/net/sf/jsqlparser/statement/alter/AlterExpression.java @@ -11,7 +11,6 @@ import java.io.Serializable; import java.util.ArrayList; -import java.util.AbstractList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -506,28 +505,49 @@ public void setUsingIfExists(boolean usingIfExists) { this.usingIfExists = usingIfExists; } - /** Returns a live view when this action has a structured PRIMARY_KEY definition. */ + /** + * 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 hasKeyIndex(Index.Kind.PRIMARY_KEY) ? new KeyColumnNames(index) : 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) { if (hasKeyIndex(Index.Kind.PRIMARY_KEY)) { - replaceKeyColumns(pkColumns); + index.setColumnsNames(pkColumns); this.pkColumns = null; } else { this.pkColumns = pkColumns; } } - /** Returns a live view when this action has a structured UNIQUE definition. */ + /** + * 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 hasKeyIndex(Index.Kind.UNIQUE) ? new KeyColumnNames(index) : 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) { if (hasKeyIndex(Index.Kind.UNIQUE)) { - replaceKeyColumns(ukColumns); + index.setColumnsNames(ukColumns); this.ukColumns = null; } else { this.ukColumns = ukColumns; @@ -538,75 +558,6 @@ private boolean hasKeyIndex(Index.Kind kind) { return index != null && index.getKind() == kind; } - private void replaceKeyColumns(List names) { - List replacement = new ArrayList<>(); - if (names != null) { - List previous = index.getColumns(); - for (int i = 0; i < names.size(); i++) { - String name = names.get(i); - if (previous != null && i < previous.size()) { - Index.ColumnParams previousColumn = previous.get(i); - if (previousColumn.toString().equals(name)) { - replacement.add(previousColumn); - continue; - } - } - replacement.add(new Index.ColumnParams(name)); - } - } - index.setColumns(replacement); - } - - /** Adapts the legacy mutable name list without copying structured expressions to strings. */ - private static class KeyColumnNames extends AbstractList { - private final Index index; - - KeyColumnNames(Index index) { - this.index = index; - } - - @Override - public String get(int position) { - Index.ColumnParams column = index.getColumns().get(position); - return column.toString(); - } - - @Override - public int size() { - return index.getColumns() == null ? 0 : index.getColumns().size(); - } - - @Override - public String set(int position, String name) { - String previous = get(position); - if (!previous.equals(name)) { - List columns = new ArrayList<>(index.getColumns()); - columns.set(position, new Index.ColumnParams(name)); - index.setColumns(columns); - } - return previous; - } - - @Override - public void add(int position, String name) { - List columns = index.getColumns() == null ? new ArrayList<>() - : new ArrayList<>(index.getColumns()); - columns.add(position, new Index.ColumnParams(name)); - index.setColumns(columns); - modCount++; - } - - @Override - public String remove(int position) { - List columns = new ArrayList<>(index.getColumns()); - Index.ColumnParams removedColumn = columns.remove(position); - String previous = removedColumn.toString(); - index.setColumns(columns); - modCount++; - return previous; - } - } - public String getUkName() { return ukName; } @@ -1443,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 index e140bd95a..ddda3cc7e 100644 --- a/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/alter/AlterKeyAccessorsTest.java @@ -13,6 +13,7 @@ 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; @@ -24,73 +25,142 @@ 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 keepsGettersSettersAndMutableListsConnectedToTheIndex(boolean primary) throws Exception { + 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(); - action.getIndex().setColumnsNames(List.of("b")); + 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.setPkColumns(List.of("c")); - } else { - action.setUkColumns(List.of("c")); - } - assertEquals(prefix + " (c)", statement.toString()); - columns(action, primary).add("d"); - columns(action, primary).set(0, "e"); - assertEquals(List.of("e", "d"), action.getIndex().getColumnsNames()); - assertEquals("e", columns(action, primary).remove(0)); - if (primary) { - action.addPkColumns("f"); + action.addPkColumns("d").addPkColumns(List.of("e")); } else { - action.addUkColumns("f"); + action.addUkColumns("d").addUkColumns(List.of("e")); } - assertEquals(prefix + " (d, f)", statement.toString()); + assertEquals(List.of("c", "d", "e"), columns(action, primary)); assertSame(originalIndex, action.getIndex()); - StringBuilder output = new StringBuilder(); - statement.accept(new StatementDeParser(output), null); - assertEquals(statement.toString(), output.toString()); - columns(action, primary).clear(); - assertTrue(action.getIndex().getColumns().isEmpty()); + assertDeparsed(statement, prefix + " (c, d, e)"); + setColumns(action, primary, null); + assertTrue(columns(action, primary).isEmpty()); + assertTrue(originalIndex.getColumns().isEmpty()); } - @Test - void preservesStructuredElementsAndIndexMetadataForUnchangedKeys() throws Exception { - Alter statement = (Alter) CCJSqlParserUtil.parse( - "ALTER TABLE t ADD UNIQUE (a, b) DEFERRABLE", - parser -> parser.withDialect( - net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect.POSTGRESQL)); + @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.setName("uq"); - Index.ColumnParams expression = - new Index.ColumnParams(new net.sf.jsqlparser.schema.Column("a")) - .withExpressionParenthesized(false); + 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); - String original = statement.toString(); - action.setUkColumns(new ArrayList<>(action.getUkColumns())); - assertSame(expression, index.getColumns().get(0)); - assertEquals(original, statement.toString()); - action.addUkColumns("c"); - assertSame(expression, index.getColumns().get(0)); - assertEquals("uq", index.getName()); - assertEquals(original.replace("(a, b)", "(a, b, c)"), statement.toString()); - assertTrue(statement.toString().endsWith("DEFERRABLE")); - assertEquals(List.of("a", "b", "c"), action.getUkColumns()); + 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 retainsLegacyOnlyConstructionAndAllowsClearingStructuredKeys() { + void retainsLegacyOnlyConstructionAndIgnoresUnrelatedIndexes() { AlterExpression action = new AlterExpression().withOperation(AlterOperation.ADD) .withPkColumns(new ArrayList<>(List.of("a"))); - action.addPkColumns("b"); + action.getPkColumns().add("b"); assertEquals("ADD PRIMARY KEY (a, b)", action.toString()); - action.setIndex(new Index().withType("PRIMARY KEY").withColumnsNames(List.of("c"))); - assertEquals(List.of("c"), action.getPkColumns()); - action.setPkColumns(null); - assertTrue(action.getPkColumns().isEmpty()); + 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()); } }