From f8a47e30b8016084dc0b0087f51176de29b58808 Mon Sep 17 00:00:00 2001 From: minleejae Date: Sat, 12 Sep 2026 12:24:32 +0900 Subject: [PATCH] fix: preserve ordered table elements through legacy list edits --- .../statement/create/table/CreateTable.java | 33 +++-- .../create/table/TableElementList.java | 140 ++++++++++++++++++ .../util/deparser/CreateTableDeParser.java | 2 +- .../create/TableElementMutationTest.java | 114 ++++++++++++++ 4 files changed, 272 insertions(+), 17 deletions(-) create mode 100644 src/main/java/net/sf/jsqlparser/statement/create/table/TableElementList.java create mode 100644 src/test/java/net/sf/jsqlparser/statement/create/TableElementMutationTest.java diff --git a/src/main/java/net/sf/jsqlparser/statement/create/table/CreateTable.java b/src/main/java/net/sf/jsqlparser/statement/create/table/CreateTable.java index 38a0134bd..c1dd7dccb 100644 --- a/src/main/java/net/sf/jsqlparser/statement/create/table/CreateTable.java +++ b/src/main/java/net/sf/jsqlparser/statement/create/table/CreateTable.java @@ -70,15 +70,19 @@ public void setUnlogged(boolean unlogged) { } /** - * @return a list of {@link ColumnDefinition}s of this table. + * @return a list of {@link ColumnDefinition}s of this table. When ordered table elements are + * present, this is a mutable view of the column definitions in that list. */ public List getColumnDefinitions() { return columnDefinitions; } public void setColumnDefinitions(List list) { - columnDefinitions = list; - tableElements = null; + if (tableElements == null) { + columnDefinitions = list; + } else { + TableElementList.replace(tableElements, ColumnDefinition.class, list); + } } public List getColumns() { @@ -137,15 +141,19 @@ public void setCreateOptionsStrings(List createOptionsStrings) { /** * @return a list of {@link Index}es (for example "PRIMARY KEY") of this table.
* Indexes created with column definitions (as in mycol INT PRIMARY KEY) are not - * inserted into this list. + * inserted into this list. When ordered table elements are present, this is a mutable + * view of their indexes. */ public List getIndexes() { return indexes; } public void setIndexes(List list) { - indexes = list; - tableElements = null; + if (tableElements == null) { + indexes = list; + } else { + TableElementList.replace(tableElements, Index.class, list); + } } /** @@ -162,15 +170,8 @@ public void setTableElements(List tableElements) { indexes = null; return; } - columnDefinitions = new ArrayList<>(); - indexes = new ArrayList<>(); - for (TableElement element : tableElements) { - if (element instanceof ColumnDefinition) { - columnDefinitions.add((ColumnDefinition) element); - } else if (element instanceof Index) { - indexes.add((Index) element); - } - } + columnDefinitions = new TableElementList<>(tableElements, ColumnDefinition.class); + indexes = new TableElementList<>(tableElements, Index.class); } /** Returns table elements of a requested AST type while preserving their declaration order. */ @@ -334,7 +335,7 @@ private void appendColumnDefinitions(StringBuilder b) { b.append(" "); b.append(PlainSelect.getStringList(columns, true, true)); } - if (tableElements != null && !tableElements.isEmpty()) { + if (tableElements != null) { b.append(" ("); b.append(PlainSelect.getStringList(tableElements, true, false)); b.append(")"); diff --git a/src/main/java/net/sf/jsqlparser/statement/create/table/TableElementList.java b/src/main/java/net/sf/jsqlparser/statement/create/table/TableElementList.java new file mode 100644 index 000000000..c221e986d --- /dev/null +++ b/src/main/java/net/sf/jsqlparser/statement/create/table/TableElementList.java @@ -0,0 +1,140 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2026 JSQLParser + * %% + * Dual licensed under GNU LGPL 2.1 or Apache License 2.0 + * #L% + */ +package net.sf.jsqlparser.statement.create.table; + +import java.io.Serializable; +import java.util.AbstractList; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.NoSuchElementException; +import java.util.Objects; + +/** A mutable, filtered view preserving the other declarations in a table definition. */ +final class TableElementList extends AbstractList + implements Serializable { + private final List elements; + private final Class type; + + TableElementList(List elements, Class type) { + this.elements = elements; + this.type = type; + } + + @Override + public int size() { + int count = 0; + for (TableElement element : elements) { + if (type.isInstance(element)) { + count++; + } + } + return count; + } + + @Override + public Iterator iterator() { + return new Iterator() { + private int cursor; + private int last = -1; + + @Override + public boolean hasNext() { + while (cursor < elements.size() && !type.isInstance(elements.get(cursor))) { + cursor++; + } + return cursor < elements.size(); + } + + @Override + public E next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + last = cursor++; + return type.cast(elements.get(last)); + } + + @Override + public void remove() { + if (last < 0) { + throw new IllegalStateException(); + } + elements.remove(last); + cursor--; + last = -1; + modCount++; + } + }; + } + + private int elementIndex(int index, boolean insertion) { + if (index < 0) { + throw new IndexOutOfBoundsException(Integer.toString(index)); + } + int count = 0; + int end = elements.size(); + for (int i = 0; i < elements.size(); i++) { + if (type.isInstance(elements.get(i))) { + if (count++ == index) { + return i; + } + end = i + 1; + } + } + if (insertion && index == count) { + return end; + } + throw new IndexOutOfBoundsException(Integer.toString(index)); + } + + @Override + public E get(int index) { + return type.cast(elements.get(elementIndex(index, false))); + } + + @Override + public E set(int index, E element) { + return type.cast(elements.set(elementIndex(index, false), Objects.requireNonNull(element))); + } + + @Override + public void add(int index, E element) { + elements.add(elementIndex(index, true), Objects.requireNonNull(element)); + modCount++; + } + + @Override + public E remove(int index) { + E removed = type.cast(elements.remove(elementIndex(index, false))); + modCount++; + return removed; + } + + static void replace(List elements, Class type, + List replacements) { + // The replacement may itself be a view of elements. + Iterator replacement = (replacements == null ? Collections.emptyList() + : new ArrayList<>(replacements)).iterator(); + ListIterator iterator = elements.listIterator(); + while (iterator.hasNext()) { + if (type.isInstance(iterator.next())) { + if (replacement.hasNext()) { + iterator.set(replacement.next()); + } else { + iterator.remove(); + } + } + } + replacement.forEachRemaining(iterator::add); + } +} diff --git a/src/main/java/net/sf/jsqlparser/util/deparser/CreateTableDeParser.java b/src/main/java/net/sf/jsqlparser/util/deparser/CreateTableDeParser.java index 8655346ea..91c0f3c2c 100644 --- a/src/main/java/net/sf/jsqlparser/util/deparser/CreateTableDeParser.java +++ b/src/main/java/net/sf/jsqlparser/util/deparser/CreateTableDeParser.java @@ -72,7 +72,7 @@ public void deParse(CreateTable createTable) { } builder.append(")"); } - if (createTable.getTableElements() != null && !createTable.getTableElements().isEmpty()) { + if (createTable.getTableElements() != null) { builder.append(" ("); for (Iterator iter = createTable.getTableElements().iterator(); iter .hasNext();) { diff --git a/src/test/java/net/sf/jsqlparser/statement/create/TableElementMutationTest.java b/src/test/java/net/sf/jsqlparser/statement/create/TableElementMutationTest.java new file mode 100644 index 000000000..08240a80c --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/create/TableElementMutationTest.java @@ -0,0 +1,114 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2026 JSQLParser + * %% + * Dual licensed under GNU LGPL 2.1 or Apache License 2.0 + * #L% + */ +package net.sf.jsqlparser.statement.create; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.statement.LikeClause; +import net.sf.jsqlparser.statement.create.table.ColDataType; +import net.sf.jsqlparser.statement.create.table.ColumnDefinition; +import net.sf.jsqlparser.statement.create.table.CreateTable; +import net.sf.jsqlparser.util.TablesNamesFinder; +import net.sf.jsqlparser.util.deparser.StatementDeParser; +import org.junit.jupiter.api.Test; + +class TableElementMutationTest { + private static final String SQL = "CREATE TABLE t (LIKE parent INCLUDING DEFAULTS, " + + "a INT, CONSTRAINT c CHECK (a > 0), b INT)"; + + @Test + void legacyListEditsUpdateOrderedElements() throws JSQLParserException { + CreateTable table = parse(); + table.getColumnDefinitions().remove(1); + table.getIndexes().clear(); + table.getColumnDefinitions().add(0, column("first")); + table.getColumnDefinitions().set(1, column("last")); + assertSql(table, "CREATE TABLE t (LIKE parent INCLUDING DEFAULTS, first INT, last INT)"); + assertEquals(3, table.getTableElements().size()); + assertTrue(new TablesNamesFinder().getTables(table).contains("parent")); + } + + @Test + void orderedListEditsRemainVisibleThroughExistingViews() throws JSQLParserException { + CreateTable table = parse(); + List columns = table.getColumnDefinitions(); + table.getTableElements().remove(1); + assertEquals("b", columns.get(0).getColumnName()); + table.getTableElements().add(column("extra")); + assertEquals(2, columns.size()); + columns.clear(); + assertEquals(2, table.getTableElements().size()); + assertEquals(1, table.getIndexes().size()); + assertEquals(1, table.getTableElements(LikeClause.class).size()); + } + + @Test + void replacingColumnsPreservesOtherElementsAndTheirPositions() throws JSQLParserException { + CreateTable table = parse(); + table.setColumnDefinitions(new ArrayList<>(table.getColumnDefinitions())); + assertSql(table, SQL); + table.setColumnDefinitions(Collections.singletonList(column("replacement"))); + assertSql(table, "CREATE TABLE t (LIKE parent INCLUDING DEFAULTS, replacement INT, " + + "CONSTRAINT c CHECK (a > 0))"); + table.setIndexes(null); + table.setColumnDefinitions(null); + assertSql(table, "CREATE TABLE t (LIKE parent INCLUDING DEFAULTS)"); + } + + @Test + void fluentAddersCanPassTheirOwnViewsToSetters() throws JSQLParserException { + CreateTable table = parse(); + table.addColumnDefinitions(column("extra")); + assertEquals(3, table.getColumnDefinitions().size()); + assertEquals(1, table.getTableElements(LikeClause.class).size()); + table.setIndexes(table.getIndexes()); + assertEquals(1, table.getIndexes().size()); + assertSql(table, SQL.substring(0, SQL.length() - 1) + ", extra INT)"); + } + + @Test + void filteredListHonorsIndexBounds() throws JSQLParserException { + List columns = parse().getColumnDefinitions(); + assertThrows(IndexOutOfBoundsException.class, () -> columns.get(-1)); + assertThrows(IndexOutOfBoundsException.class, () -> columns.get(2)); + assertThrows(IndexOutOfBoundsException.class, () -> columns.add(3, column("bad"))); + } + + @Test + void clearingAllElementsKeepsBothRenderersConsistent() throws JSQLParserException { + CreateTable table = parse(); + table.getTableElements().clear(); + assertSql(table, "CREATE TABLE t ()"); + assertTrue(table.getColumnDefinitions().isEmpty()); + assertTrue(table.getIndexes().isEmpty()); + } + + private static CreateTable parse() throws JSQLParserException { + return (CreateTable) CCJSqlParserUtil.parse(SQL); + } + + private static ColumnDefinition column(String name) { + return new ColumnDefinition(name, new ColDataType("INT")); + } + + private static void assertSql(CreateTable table, String expected) { + assertEquals(expected, table.toString()); + StringBuilder buffer = new StringBuilder(); + table.accept(new StatementDeParser(buffer), null); + assertEquals(expected, buffer.toString()); + } +}