From 09e7250378262062dfde1dc15f7568108c82f32f Mon Sep 17 00:00:00 2001 From: minleejae Date: Thu, 1 Oct 2026 22:36:56 +0900 Subject: [PATCH 1/2] refactor: share CREATE INDEX options with its index definition Signed-off-by: minleejae --- .../statement/create/index/CreateIndex.java | 81 ++++++-- .../util/TableDefinitionTraversal.java | 1 - .../create/CreateIndexOptionStateTest.java | 181 ++++++++++++++++++ 3 files changed, 242 insertions(+), 21 deletions(-) create mode 100644 src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java diff --git a/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java b/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java index 415e4800d..5846c9462 100644 --- a/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java +++ b/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java @@ -20,16 +20,13 @@ public class CreateIndex implements Statement { private Table table; private Index index; + private Index detachedOptions; private List tailParameters; private boolean indexTypeBeforeOn = false; private boolean usingIfNotExists = false; private boolean concurrently; private boolean only; private boolean nullFiltered; - private List includeColumns; - private Boolean nullsDistinct; - private List storageParameters; - private String tableSpace; private Expression where; public boolean isIndexTypeBeforeOn() { @@ -80,35 +77,53 @@ public CreateIndex withNullFiltered(boolean nullFiltered) { } public List getIncludeColumns() { - return includeColumns; + Index options = getOptions(); + return options == null ? null : options.getIncludeColumns(); } public void setIncludeColumns(List includeColumns) { - this.includeColumns = includeColumns; + getOrCreateOptions().setIncludeColumns(includeColumns); } public Boolean getNullsDistinct() { - return nullsDistinct; + Index options = getOptions(); + return options == null ? null : options.getNullsDistinct(); } public void setNullsDistinct(Boolean nullsDistinct) { - this.nullsDistinct = nullsDistinct; + getOrCreateOptions().setNullsDistinct(nullsDistinct); } public List getStorageParameters() { - return storageParameters; + Index options = getOptions(); + return options == null ? null : options.getStorageParameters(); } public void setStorageParameters(List storageParameters) { - this.storageParameters = storageParameters; + getOrCreateOptions().setStorageParameters(storageParameters); } public String getTableSpace() { - return tableSpace; + Index options = getOptions(); + return options == null ? null : options.getTableSpace(); } public void setTableSpace(String tableSpace) { - this.tableSpace = tableSpace; + getOrCreateOptions().setTableSpace(tableSpace); + } + + private Index getOptions() { + return index == null ? detachedOptions : index; + } + + private Index getOrCreateOptions() { + if (index != null) { + return index; + } + if (detachedOptions == null) { + detachedOptions = new Index(); + } + return detachedOptions; } public Expression getWhere() { @@ -128,7 +143,33 @@ public Index getIndex() { return index; } + /** + * Replaces the index definition. Options supplied by the new index take precedence; omitted + * options inherit the current statement options, including those set before an index was + * attached. Passing null detaches the definition without discarding its options. Use the option + * setters with null to clear individual options. + */ public void setIndex(Index index) { + Index previousOptions = getOptions(); + if (index == null) { + detachedOptions = previousOptions; + } else { + if (previousOptions != null && previousOptions != index) { + if (index.getIncludeColumns() == null) { + index.setIncludeColumns(previousOptions.getIncludeColumns()); + } + if (index.getNullsDistinct() == null) { + index.setNullsDistinct(previousOptions.getNullsDistinct()); + } + if (index.getStorageParameters() == null) { + index.setStorageParameters(previousOptions.getStorageParameters()); + } + if (index.getTableSpace() == null) { + index.setTableSpace(previousOptions.getTableSpace()); + } + } + detachedOptions = null; + } this.index = index; } @@ -225,18 +266,18 @@ private void appendIndexColumns(StringBuilder buffer, Consumer expre private void appendPostgreSqlTail(StringBuilder buffer, Consumer expressionPrinter) { - if (includeColumns != null) { - buffer.append(" INCLUDE (").append(String.join(", ", includeColumns)).append(")"); + if (getIncludeColumns() != null) { + buffer.append(" INCLUDE (").append(String.join(", ", getIncludeColumns())).append(")"); } - if (nullsDistinct != null) { - buffer.append(" NULLS ").append(nullsDistinct ? "DISTINCT" : "NOT DISTINCT"); + if (getNullsDistinct() != null) { + buffer.append(" NULLS ").append(getNullsDistinct() ? "DISTINCT" : "NOT DISTINCT"); } - if (storageParameters != null) { + if (getStorageParameters() != null) { buffer.append(" WITH "); - Index.Option.appendListTo(buffer, storageParameters, expressionPrinter); + Index.Option.appendListTo(buffer, getStorageParameters(), expressionPrinter); } - if (tableSpace != null) { - buffer.append(" TABLESPACE ").append(tableSpace); + if (getTableSpace() != null) { + buffer.append(" TABLESPACE ").append(getTableSpace()); } if (where != null) { buffer.append(" WHERE "); diff --git a/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java b/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java index ec7ab180e..574a2612b 100644 --- a/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java +++ b/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java @@ -44,7 +44,6 @@ public static void visit(CreateIndex createIndex, Consumer expressio if (createIndex.getIndex() != null) { visit(createIndex.getIndex(), expressions, tables); } - visitOptions(createIndex.getStorageParameters(), expressions); accept(createIndex.getWhere(), expressions); } diff --git a/src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java b/src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java new file mode 100644 index 000000000..d0bdc38c9 --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java @@ -0,0 +1,181 @@ +/*- + * #%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.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; + +import java.util.ArrayList; +import java.util.List; +import net.sf.jsqlparser.expression.ExpressionVisitorAdapter; +import net.sf.jsqlparser.expression.LongValue; +import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.schema.Table; +import net.sf.jsqlparser.statement.StatementVisitorAdapter; +import net.sf.jsqlparser.statement.create.index.CreateIndex; +import net.sf.jsqlparser.statement.create.table.Index; +import net.sf.jsqlparser.statement.select.SelectVisitorAdapter; +import net.sf.jsqlparser.util.deparser.ExpressionDeParser; +import net.sf.jsqlparser.util.deparser.SelectDeParser; +import net.sf.jsqlparser.util.deparser.StatementDeParser; +import org.junit.jupiter.api.Test; + +class CreateIndexOptionStateTest { + private static final String SQL = "CREATE UNIQUE INDEX ix ON t (id) INCLUDE (payload) " + + "NULLS NOT DISTINCT WITH (fillfactor = 80) TABLESPACE fast_space"; + + @Test + void parsedOptionsAreSharedAndIndexEditsReachBothRenderers() throws Exception { + CreateIndex statement = parse(SQL); + Index index = statement.getIndex(); + assertSame(statement.getIncludeColumns(), index.getIncludeColumns()); + assertSame(statement.getStorageParameters(), index.getStorageParameters()); + assertEquals(Boolean.FALSE, index.getNullsDistinct()); + assertEquals("fast_space", index.getTableSpace()); + + index.getIncludeColumns().add("extra"); + index.setNullsDistinct(true); + index.getStorageParameters().get(0).setValue(new LongValue(90)); + index.setTableSpace("other_space"); + + assertEquals(List.of("payload", "extra"), statement.getIncludeColumns()); + assertEquals(Boolean.TRUE, statement.getNullsDistinct()); + assertEquals("other_space", statement.getTableSpace()); + assertRoundTrip("CREATE UNIQUE INDEX ix ON t (id) INCLUDE (payload, extra) " + + "NULLS DISTINCT WITH (fillfactor = 90) TABLESPACE other_space", statement); + } + + @Test + void statementSettersAndClearsUpdateTheIndexDefinition() throws Exception { + CreateIndex statement = parse("CREATE UNIQUE INDEX ix ON t (id)"); + statement.setIncludeColumns(List.of("payload")); + statement.setNullsDistinct(false); + statement.setStorageParameters(List.of(option(80))); + statement.setTableSpace("fast_space"); + Index index = statement.getIndex(); + assertSame(statement.getIncludeColumns(), index.getIncludeColumns()); + assertSame(statement.getStorageParameters(), index.getStorageParameters()); + assertEquals(Boolean.FALSE, index.getNullsDistinct()); + assertEquals("fast_space", index.getTableSpace()); + assertRoundTrip(SQL, statement); + + statement.setIncludeColumns(null); + statement.setNullsDistinct(null); + statement.setStorageParameters(null); + statement.setTableSpace(null); + assertNull(index.getIncludeColumns()); + assertNull(index.getNullsDistinct()); + assertNull(index.getStorageParameters()); + assertNull(index.getTableSpace()); + assertRoundTrip("CREATE UNIQUE INDEX ix ON t (id)", statement); + } + + @Test + void optionsCanBeConfiguredBeforeTheIndexWithoutLosingSuppliedOptions() throws Exception { + CreateIndex statement = new CreateIndex().withTable(new Table("t")) + .withIncludeColumns(List.of("payload")) + .withNullsDistinct(false) + .withStorageParameters(List.of(option(80))) + .withTableSpace("staged_space"); + assertNull(statement.getIndex()); + assertEquals("staged_space", statement.getTableSpace()); + + Index index = new Index().withType("UNIQUE").withName("ix") + .withColumnsNames(List.of("id")); + index.setTableSpace("fast_space"); + statement.setIndex(index); + + assertSame(index, statement.getIndex()); + assertSame(statement.getStorageParameters(), index.getStorageParameters()); + assertRoundTrip(SQL, statement); + } + + @Test + void replacementOptionsTakePrecedenceAndMissingOptionsSurviveDetachment() throws Exception { + CreateIndex statement = parse(SQL); + Index replacement = new Index().withType("UNIQUE").withName("replacement") + .withColumnsNames(List.of("id")); + replacement.setIncludeColumns(List.of("extra")); + replacement.setStorageParameters(List.of(option(90))); + statement.setIndex(replacement); + statement.setIndex(replacement); + assertSame(replacement.getIncludeColumns(), statement.getIncludeColumns()); + assertRoundTrip("CREATE UNIQUE INDEX replacement ON t (id) INCLUDE (extra) " + + "NULLS NOT DISTINCT WITH (fillfactor = 90) TABLESPACE fast_space", statement); + + statement.setIndex(null); + statement.setIndex(null); + assertNull(statement.getIndex()); + assertEquals(List.of("extra"), statement.getIncludeColumns()); + assertEquals(Boolean.FALSE, statement.getNullsDistinct()); + assertEquals("fast_space", statement.getTableSpace()); + statement.setTableSpace(null); + Index reattached = new Index().withType("UNIQUE").withName("reattached") + .withColumnsNames(List.of("id")); + statement.setIndex(reattached); + assertSame(reattached.getStorageParameters(), statement.getStorageParameters()); + assertNull(reattached.getTableSpace()); + assertRoundTrip("CREATE UNIQUE INDEX reattached ON t (id) INCLUDE (extra) " + + "NULLS NOT DISTINCT WITH (fillfactor = 90)", statement); + } + + @Test + void indexOptionEditsAreVisitedOnceAndCanBeDeparsedThroughCustomVisitors() throws Exception { + CreateIndex statement = parse(SQL); + statement.getIndex().setStorageParameters(List.of(option(90))); + List values = new ArrayList<>(); + ExpressionVisitorAdapter expressions = new ExpressionVisitorAdapter() { + @Override + public Void visit(LongValue value, S context) { + assertEquals("context", context); + values.add(value.getValue()); + return null; + } + }; + statement.accept(new StatementVisitorAdapter<>(new SelectVisitorAdapter<>(expressions)), + "context"); + assertEquals(List.of(90L), values); + + StringBuilder output = new StringBuilder(); + ExpressionDeParser deparser = new ExpressionDeParser() { + @Override + public StringBuilder visit(LongValue value, S context) { + return getBuilder().append(value.getValue() + 1); + } + }; + statement.accept(new StatementDeParser(deparser, new SelectDeParser(), output)); + assertEquals(SQL.replace("80", "91"), output.toString()); + assertEquals(output.toString(), parse(output.toString()).toString()); + assertEquals(SQL.replace("80", "90"), statement.toString()); + } + + private static Index.Option option(long value) { + return new Index.Option("fillfactor", new LongValue(value), true); + } + + private static CreateIndex parse(String sql) throws Exception { + return (CreateIndex) CCJSqlParserUtil.parse(sql, + parser -> parser.withDialect(Dialect.POSTGRESQL)); + } + + private static void assertRoundTrip(String expected, CreateIndex statement) throws Exception { + assertEquals(expected, statement.toString()); + StringBuilder output = new StringBuilder(); + statement.accept(new StatementDeParser(output)); + assertEquals(expected, output.toString()); + CreateIndex reparsed = parse(expected); + assertEquals(expected, reparsed.toString()); + assertEquals(statement.getIncludeColumns(), reparsed.getIndex().getIncludeColumns()); + assertEquals(statement.getNullsDistinct(), reparsed.getIndex().getNullsDistinct()); + assertEquals(statement.getTableSpace(), reparsed.getIndex().getTableSpace()); + } +} From 14d8fff39d1dcb78360600a7f673329cdc841607 Mon Sep 17 00:00:00 2001 From: minleejae Date: Thu, 1 Oct 2026 22:55:14 +0900 Subject: [PATCH 2/2] Preserve detached CREATE INDEX option isolation and traversal Signed-off-by: minleejae --- .../statement/create/index/CreateIndex.java | 20 ++++++- .../util/TableDefinitionTraversal.java | 2 + .../create/CreateIndexOptionStateTest.java | 58 +++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java b/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java index 5846c9462..11ea13682 100644 --- a/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java +++ b/src/main/java/net/sf/jsqlparser/statement/create/index/CreateIndex.java @@ -81,6 +81,10 @@ public List getIncludeColumns() { return options == null ? null : options.getIncludeColumns(); } + /** + * Copies the supplied list as {@link Index#setIncludeColumns(List)} does; null clears it. + * {@link #getIncludeColumns()} returns the live, mutable list held by the index options. + */ public void setIncludeColumns(List includeColumns) { getOrCreateOptions().setIncludeColumns(includeColumns); } @@ -99,6 +103,11 @@ public List getStorageParameters() { return options == null ? null : options.getStorageParameters(); } + /** + * Copies the list container as {@link Index#setStorageParameters(List)} does, retaining the + * option objects; null clears it. {@link #getStorageParameters()} returns the live, mutable + * list held by the index options. + */ public void setStorageParameters(List storageParameters) { getOrCreateOptions().setStorageParameters(storageParameters); } @@ -146,13 +155,20 @@ public Index getIndex() { /** * Replaces the index definition. Options supplied by the new index take precedence; omitted * options inherit the current statement options, including those set before an index was - * attached. Passing null detaches the definition without discarding its options. Use the option + * attached. Passing null detaches the definition without discarding its options. The detached + * option lists have independent containers, with their elements retained. Use the option * setters with null to clear individual options. */ public void setIndex(Index index) { Index previousOptions = getOptions(); if (index == null) { - detachedOptions = previousOptions; + if (this.index != null) { + detachedOptions = new Index(); + detachedOptions.setIncludeColumns(previousOptions.getIncludeColumns()); + detachedOptions.setNullsDistinct(previousOptions.getNullsDistinct()); + detachedOptions.setStorageParameters(previousOptions.getStorageParameters()); + detachedOptions.setTableSpace(previousOptions.getTableSpace()); + } } else { if (previousOptions != null && previousOptions != index) { if (index.getIncludeColumns() == null) { diff --git a/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java b/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java index 574a2612b..2f4e1939e 100644 --- a/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java +++ b/src/main/java/net/sf/jsqlparser/util/TableDefinitionTraversal.java @@ -43,6 +43,8 @@ public static void visit(CreateIndex createIndex, Consumer expressio accept(createIndex.getTable(), tables); if (createIndex.getIndex() != null) { visit(createIndex.getIndex(), expressions, tables); + } else { + visitOptions(createIndex.getStorageParameters(), expressions); } accept(createIndex.getWhere(), expressions); } diff --git a/src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java b/src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java index d0bdc38c9..3951f3a85 100644 --- a/src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/create/CreateIndexOptionStateTest.java @@ -10,6 +10,7 @@ package net.sf.jsqlparser.statement.create; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; @@ -128,6 +129,63 @@ void replacementOptionsTakePrecedenceAndMissingOptionsSurviveDetachment() throws + "NULLS NOT DISTINCT WITH (fillfactor = 90)", statement); } + @Test + void detachedOptionSettersDoNotMutateTheRemovedIndex() throws Exception { + CreateIndex statement = parse(SQL); + Index removed = statement.getIndex(); + statement.setIndex(null); + + assertNotSame(removed.getIncludeColumns(), statement.getIncludeColumns()); + assertNotSame(removed.getStorageParameters(), statement.getStorageParameters()); + assertSame(removed.getStorageParameters().get(0), statement.getStorageParameters().get(0)); + List detachedColumns = statement.getIncludeColumns(); + List detachedParameters = statement.getStorageParameters(); + statement.setIndex(null); + assertSame(detachedColumns, statement.getIncludeColumns()); + assertSame(detachedParameters, statement.getStorageParameters()); + statement.getIncludeColumns().add("extra"); + assertEquals(List.of("payload", "extra"), statement.getIncludeColumns()); + assertEquals(List.of("payload"), removed.getIncludeColumns()); + + statement.setTableSpace("detached_space"); + statement.setNullsDistinct(true); + statement.setIncludeColumns(List.of("detached_payload")); + statement.setStorageParameters(List.of(option(90))); + assertEquals("fast_space", removed.getTableSpace()); + assertEquals(Boolean.FALSE, removed.getNullsDistinct()); + assertEquals(List.of("payload"), removed.getIncludeColumns()); + assertEquals("80", removed.getStorageParameters().get(0).getValue().toString()); + + removed.setTableSpace("external_space"); + removed.setNullsDistinct(null); + assertEquals("detached_space", statement.getTableSpace()); + assertEquals(Boolean.TRUE, statement.getNullsDistinct()); + statement.setIndex(new Index().withType("UNIQUE").withName("reattached") + .withColumnsNames(List.of("id"))); + assertRoundTrip("CREATE UNIQUE INDEX reattached ON t (id) INCLUDE (detached_payload) " + + "NULLS DISTINCT WITH (fillfactor = 90) TABLESPACE detached_space", statement); + } + + @Test + void optionsAreVisitedBeforeAttachmentAndAfterDetachment() throws Exception { + for (CreateIndex statement : List.of(new CreateIndex(), parse(SQL))) { + statement.setIndex(null); + statement.setStorageParameters(List.of(option(80))); + List values = new ArrayList<>(); + ExpressionVisitorAdapter expressions = new ExpressionVisitorAdapter() { + @Override + public Void visit(LongValue value, S context) { + assertEquals("context", context); + values.add(value.getValue()); + return null; + } + }; + statement.accept(new StatementVisitorAdapter<>(new SelectVisitorAdapter<>(expressions)), + "context"); + assertEquals(List.of(80L), values); + } + } + @Test void indexOptionEditsAreVisitedOnceAndCanBeDeparsedThroughCustomVisitors() throws Exception { CreateIndex statement = parse(SQL);