Unify column option storage and mutation contracts (#2769)

Signed-off-by: minleejae <mmj9808@gmail.com>

Author: minleejae

src/main/java/net/sf/jsqlparser/statement/create/table/ColumnDefinition.java

@@ -10,7 +10,6 @@
 package net.sf.jsqlparser.statement.create.table;
 
 import net.sf.jsqlparser.statement.imprt.ImportColumn;
-import net.sf.jsqlparser.statement.select.PlainSelect;
 import net.sf.jsqlparser.expression.Expression;
 
 import java.io.Serializable;
@@ -19,7 +18,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Optional;
+import java.util.Objects;
 import java.util.function.Consumer;
 
 /**
@@ -29,7 +28,6 @@ public class ColumnDefinition implements ImportColumn, TableElement, Serializabl
 
     private String columnName;
     private ColDataType colDataType;
-    private List<String> columnSpecs;
     private List<ColumnOption> columnOptions;
     private boolean withOptions;
 
@@ -50,42 +48,54 @@ public ColumnDefinition(String columnName, ColDataType colDataType) {
 
     public ColumnDefinition(String columnName, ColDataType colDataType, List<String> columnSpecs) {
         this(columnName, colDataType);
-        this.columnSpecs = columnSpecs;
+        columnOptions = rawOptions(columnSpecs);
     }
 
     /**
-     * Returns raw specifications, or a token snapshot when structured options are present. Use the
-     * option API or {@link #addColumnSpecs(Collection)} to append without discarding structured
-     * expressions, references and constraints. DEFAULT values use the expression's SQL rendering.
+     * Returns an unmodifiable token snapshot for both raw and structured options, or null when
+     * options are unset. Use {@link #getColumnOptions()} to edit the nodes, or
+     * {@link #addColumnSpecs(Collection)} to append without discarding structured expressions,
+     * references and constraints. DEFAULT values use the expression's SQL rendering.
      */
     public List<String> getColumnSpecs() {
         if (columnOptions != null) {
             List<String> tokens = new ArrayList<>();
             for (ColumnOption option : columnOptions) {
                 tokens.addAll(option.getTokens());
             }
-            return tokens;
+            return Collections.unmodifiableList(tokens);
         }
-        return columnSpecs;
+        return null;
     }
 
+    /**
+     * Replaces every option with opaque tokens, without parsing them. The supplied list is copied;
+     * null clears the options, while an empty list stores an explicitly empty option list.
+     */
     public void setColumnSpecs(List<String> list) {
-        columnSpecs = list;
-        columnOptions = null;
+        columnOptions = rawOptions(list);
+    }
+
+    private static List<ColumnOption> rawOptions(List<String> list) {
+        List<ColumnOption> options = list == null ? null : new ArrayList<>();
+        if (list != null && !list.isEmpty()) {
+            options.add(ColumnOption.raw(new ArrayList<>(list)));
+        }
+        return options;
     }
 
     /**
-     * Returns column options in source order, including defaults, references, generated columns,
-     * nullability, collation, comments, visibility and MySQL attributes. Unrecognized options
-     * remain raw. Structured option keywords use canonical capitalization when rendered.
+     * Returns the mutable option list in source order, or null when options are unset. Opaque
+     * specifications are included as {@link ColumnOption.Kind#OTHER} nodes. Structured option
+     * keywords use canonical capitalization when rendered.
      */
     public List<ColumnOption> getColumnOptions() {
         return columnOptions;
     }
 
+    /** Copies the list membership while retaining the supplied mutable option nodes. */
     public void setColumnOptions(List<ColumnOption> columnOptions) {
-        this.columnOptions = columnOptions;
-        this.columnSpecs = null;
+        this.columnOptions = columnOptions == null ? null : new ArrayList<>(columnOptions);
     }
 
     public boolean isSerialDefaultValue() {
@@ -110,13 +120,12 @@ public ColumnDefinition withColumnOptions(List<ColumnOption> columnOptions) {
     }
 
     public ColumnDefinition addColumnOptions(ColumnOption... columnOptions) {
-        List<ColumnOption> collection =
-                Optional.ofNullable(getColumnOptions()).orElseGet(ArrayList::new);
-        if (this.columnOptions == null && columnSpecs != null && !columnSpecs.isEmpty()) {
-            collection.add(ColumnOption.raw(new ArrayList<>(columnSpecs)));
+        Objects.requireNonNull(columnOptions, "columnOptions");
+        if (this.columnOptions == null) {
+            this.columnOptions = new ArrayList<>();
         }
-        Collections.addAll(collection, columnOptions);
-        return withColumnOptions(collection);
+        Collections.addAll(this.columnOptions, columnOptions);
+        return this;
     }
 
     public ColDataType getColDataType() {
@@ -170,8 +179,6 @@ protected void appendDataTypeAndSpecTo(StringBuilder builder,
                 builder.append(' ');
                 option.appendTo(builder, expressionPrinter);
             }
-        } else if (columnSpecs != null && !columnSpecs.isEmpty()) {
-            builder.append(' ').append(PlainSelect.getStringList(columnSpecs, false, false));
         }
     }
 
@@ -194,15 +201,15 @@ public ColumnDefinition addColumnSpecs(String... columnSpecs) {
         return addColumnSpecs(Arrays.asList(columnSpecs));
     }
 
+    /** Appends copied opaque tokens after the existing nodes, preserving their structure. */
     public ColumnDefinition addColumnSpecs(Collection<String> columnSpecs) {
-        if (columnOptions != null) {
-            if (!columnSpecs.isEmpty()) {
-                columnOptions.add(ColumnOption.raw(new ArrayList<>(columnSpecs)));
-            }
-            return this;
+        Objects.requireNonNull(columnSpecs, "columnSpecs");
+        if (columnOptions == null) {
+            columnOptions = new ArrayList<>();
         }
-        List<String> collection = Optional.ofNullable(getColumnSpecs()).orElseGet(ArrayList::new);
-        collection.addAll(columnSpecs);
-        return this.withColumnSpecs(collection);
+        if (!columnSpecs.isEmpty()) {
+            columnOptions.add(ColumnOption.raw(new ArrayList<>(columnSpecs)));
+        }
+        return this;
     }
 }

src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt

@@ -2080,15 +2080,6 @@ public class CCJSqlParser extends AbstractJSqlParser<CCJSqlParser> {
         }
     }
 
-    private static boolean hasStructuredColumnOption(List<ColumnOption> options) {
-        for (ColumnOption option : options) {
-            if (option.getKind() != ColumnOption.Kind.OTHER) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     /** Builds names from lexer-separated components without reinterpreting PostgreSQL dots. */
     protected Table createTable(List<String> names) {
         return Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect)) && names.size() == 1
@@ -14538,23 +14529,19 @@ ColumnDefinition ColumnDefinition(): {
     ColumnDefinition coldef;
     String columnName;
     ColDataType colDataType;
-    List<String> columnSpecs = new ArrayList<String>();
     List<ColumnOption> columnOptions = new ArrayList<ColumnOption>();
     ColumnOption option;
 } {
     columnName=RelObjectName()
     colDataType=ColDataType()
     ( LOOKAHEAD(2) option=ColumnDefinitionOption() {
         columnOptions.add(option);
-        columnSpecs.addAll(option.getTokens());
     } )*
     {
         coldef = new ColumnDefinition();
         coldef.setColumnName(columnName);
         coldef.setColDataType(colDataType);
-        if (columnSpecs.size() > 0)
-            coldef.setColumnSpecs(columnSpecs);
-        if (hasStructuredColumnOption(columnOptions))
+        if (!columnOptions.isEmpty())
             coldef.setColumnOptions(columnOptions);
         return coldef;
     }
@@ -17271,7 +17258,6 @@ AlterExpression.ColumnDataType AlterExpressionColumnDataType():
     boolean withType = false;
     boolean useSetData = false;
     ColDataType dataType = null;
-    List<String> columnSpecs = new ArrayList<String>();
     List<ColumnOption> columnOptions = new ArrayList<ColumnOption>();
     ColumnOption option = null;
     AlterExpression.ColumnDataType result;
@@ -17293,19 +17279,16 @@ AlterExpression.ColumnDataType AlterExpressionColumnDataType():
     ( LOOKAHEAD(2) dataType = ColDataType() )?
     ( LOOKAHEAD(2, { !(withType && getToken(1).kind == K_USING) }) option = ColumnDefinitionOption() {
         columnOptions.add(option);
-        columnSpecs.addAll(option.getTokens());
     } )*
     [ LOOKAHEAD({ withType && getToken(1).kind == K_USING }) <K_USING> usingExpression=Expression() ]
     )
     {
         requireDdlSyntax(!withType || dataType != null, "TYPE requires a data type");
-        result = new AlterExpression.ColumnDataType(columnName, withType, dataType, columnSpecs);
+        result = new AlterExpression.ColumnDataType(columnName, withType, dataType, null);
         result.setUseSetData(useSetData);
         result.setUsingExpression(usingExpression);
         result.setIdentityAlterations(identityAlterations);
-        if (hasStructuredColumnOption(columnOptions)) {
-            result.setColumnOptions(columnOptions);
-        }
+        result.setColumnOptions(columnOptions);
         return result;
     }
 }

src/site/sphinx/usage.rst

@@ -652,11 +652,36 @@ Charset remains in ``ColDataType.getCharacterSet()``; column collation is in its
 COLLATE option. Consumers can map both directly without reconstructing tokens.
 StringValue comment bodies retain their SQL escape representation.
 
-The legacy ``getColumnSpecs()`` returns a token snapshot when options are
-structured; expression fragments may occupy one token and structured keywords
-use canonical capitalization. Mutate the option objects to change the AST.
-``addColumnSpecs`` preserves existing options; ``setColumnSpecs`` explicitly
-replaces them with raw specifications.
+``ColumnDefinition`` stores all options in one ordered list, including opaque
+``ColumnOption.Kind.OTHER`` nodes. ``getColumnOptions()`` returns that mutable
+list. ``getColumnSpecs()`` returns an **unmodifiable snapshot** for both raw and
+structured options; expression fragments may occupy one token and structured
+keywords use canonical capitalization. Unset options return null; an explicitly
+empty list returns an empty snapshot.
+
+.. code-block:: java
+
+    CreateTable table = (CreateTable) CCJSqlParserUtil.parse(
+            "CREATE TABLE t (id INT DEFAULT 1)");
+    ColumnDefinition column = table.getColumnDefinitions().get(0);
+    column.getColumnOptions().get(0).setDefaultExpression(new LongValue(2));
+    column.addColumnSpecs("NOT", "NULL"); // keeps the structured DEFAULT node
+    // CREATE TABLE t (id INT DEFAULT 2 NOT NULL)
+
+Migration: calls such as ``getColumnSpecs().add(...)`` or ``.set(...)`` now throw
+``UnsupportedOperationException`` instead of sometimes changing the AST and
+sometimes editing a discarded copy. Use ``addColumnSpecs`` to append raw tokens,
+edit the option nodes directly, or pass a modified token-list copy to
+``setColumnSpecs`` to replace every option with opaque text. Raw tokens are never
+automatically parsed. Consequently ``getColumnOptions()`` is also non-null for
+raw specifications; inspect ``Kind`` to distinguish them.
+
+The constructor and ``setColumnSpecs`` copy supplied tokens. ``setColumnOptions``
+copies list membership but retains the mutable child nodes; later changes to the
+caller's list do not change the column. Both getters return null after either
+setter receives null. Appending preserves the current nodes and accepts an
+immutable input list. An empty append preserves existing options and creates an
+empty option list when none was set.
 
 Inspect logical replication statements
 ======================================

src/test/java/net/sf/jsqlparser/statement/create/ColumnAttributeTest.java

@@ -156,7 +156,8 @@ void mutationsUpdateLegacyTokensAndBothRenderers() throws Exception {
         column(statement).addColumnSpecs("CUSTOM");
         assertEquals(ColumnOption.Kind.OTHER, options.get(options.size() - 1).getKind());
         column(statement).setColumnSpecs(List.of("NOT", "NULL"));
-        assertNull(column(statement).getColumnOptions());
+        assertEquals(ColumnOption.Kind.OTHER,
+                column(statement).getColumnOptions().get(0).getKind());
         assertEquals("CREATE TABLE t (c INT NOT NULL)", statement.toString());
     }
 

src/test/java/net/sf/jsqlparser/statement/create/ColumnDefaultExpressionTest.java

@@ -130,7 +130,7 @@ void replacingDefaultUpdatesLegacyTokensAndBothRenderers(String sql)
         assertNotNull(defaultOption(column).getDefaultExpression());
         assertRoundTrip(statement);
         column.setColumnSpecs(Arrays.asList("DEFAULT", "9"));
-        assertNull(column.getColumnOptions());
+        assertEquals(ColumnOption.Kind.OTHER, column.getColumnOptions().get(0).getKind());
         assertEquals(sql.replace("(1 + 2) NOT NULL", "9"), statement.toString());
         assertRoundTrip(statement);
     }