feat: reuse table constraint grammar for PostgreSQL ALTER ADD (#2612)

Author: minleejae

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

@@ -1547,6 +1547,45 @@ public class CCJSqlParser extends AbstractJSqlParser<CCJSqlParser> {
         return false;
     }
 
+    /** Keeps legacy ALTER key accessors populated from the common structured definition. */
+    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());
+            alterExp.setUkName(index instanceof NamedConstraint
+                    ? ((NamedConstraint) index).getIndexName() : index.getName());
+            alterExp.setUk(index.getType().toUpperCase(Locale.ROOT).contains("KEY"));
+            alterExp.setUkTypeSpecified(index.getIndexKeyword() != null);
+            for (String option : new ArrayList<String>(index.getIndexSpec())) {
+                if (option.toUpperCase(Locale.ROOT).startsWith("USING ")) {
+                    alterExp.addParameters("USING");
+                    alterExp.addParameters(option.substring("USING ".length()));
+                    index.getIndexSpec().remove(option);
+                } else if (option.toUpperCase(Locale.ROOT).startsWith("COMMENT ")) {
+                    index.setCommentText(option.substring("COMMENT ".length()));
+                    index.getIndexSpec().remove(option);
+                }
+            }
+        }
+        if (index instanceof ForeignKeyIndex) {
+            ForeignKeyIndex foreignKey = (ForeignKeyIndex) index;
+            alterExp.setFkColumns(foreignKey.getColumnsNames());
+            if (foreignKey.getTable() != null) {
+                alterExp.setFkSourceSchema(foreignKey.getTable().getSchemaName());
+                alterExp.setFkSourceTable(foreignKey.getTable().getName());
+            }
+            alterExp.setFkSourceColumns(foreignKey.getReferencedColumnNames());
+            for (ReferentialAction.Type type : ReferentialAction.Type.values()) {
+                ReferentialAction action = foreignKey.getReferentialAction(type);
+                if (action != null) {
+                    alterExp.setReferentialAction(action.getType(), action.getAction());
+                }
+            }
+        }
+    }
+
     private static boolean hasStructuredColumnOption(List<ColumnOption> options) {
         for (ColumnOption option : options) {
             if (option.getKind() != ColumnOption.Kind.OTHER) {
@@ -13155,6 +13194,11 @@ void PostgreSqlConstraintAttributes(Index index):
  * to {@link #TableIndexSpec(boolean)} so CREATE and ALTER expose the same structured AST.
  */
 Index CreateTableConstraint():
+{ Index index; }
+{ index=TableConstraint(true) { return index; } }
+
+/** Shared table-constraint body; index option boundaries depend on CREATE versus ALTER. */
+Index TableConstraint(boolean createContext):
 {
     String constraintName = null;
     Index index = null;
@@ -13163,13 +13207,13 @@ Index CreateTableConstraint():
 }
 {
     (
-        LOOKAHEAD({ isTableIndexAhead() }) index=TableIndexSpec(true)
+        LOOKAHEAD({ isTableIndexAhead() }) index=TableIndexSpec(createContext)
         |
         <K_CONSTRAINT>
         [ LOOKAHEAD({ !isTableIndexAhead() && getToken(1).kind != K_FOREIGN
                 && getToken(1).kind != K_CHECK && getToken(1).kind != K_EXCLUDE }) constraintName=RelObjectName() ]
         (
-            LOOKAHEAD({ isTableIndexAhead() }) index=TableIndexSpec(true) {
+            LOOKAHEAD({ isTableIndexAhead() }) index=TableIndexSpec(createContext) {
                 if (index instanceof NamedConstraint) {
                     ((NamedConstraint) index).setUseConstraintKeyword(true);
                     index.setName(constraintName);
@@ -13210,7 +13254,7 @@ ExcludeConstraint PostgreSqlExcludeConstraint():
 {
     <K_EXCLUDE>
     [ LOOKAHEAD(2) <K_USING> using=RelObjectName() { constraint.setUsing(using); } ]
-    [ "(" column=PostgreSqlExcludeElement() { columns.add(column); }
+    [ LOOKAHEAD(2) "(" column=PostgreSqlExcludeElement() { columns.add(column); }
         ( "," column=PostgreSqlExcludeElement() { columns.add(column); } )* ")"
         { constraint.setColumns(columns); } ]
     PostgreSqlConstraintOptions(constraint)
@@ -15415,27 +15459,15 @@ AlterExpression AlterExpressionAddAlterModify():
             alterExp.setIndex(index);
         }
         |
+        LOOKAHEAD({ alterExp.getOperation() == AlterOperation.ADD
+                && Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect))
+                && (getToken(1).kind == K_CONSTRAINT || getToken(1).kind == K_PRIMARY
+                    || getToken(1).kind == K_UNIQUE || getToken(1).kind == K_FOREIGN
+                    || getToken(1).kind == K_CHECK || getToken(1).kind == K_EXCLUDE) })
+        index=TableConstraint(false) { setAlterTableIndex(alterExp, index); }
+        |
         LOOKAHEAD({ isTableIndexAhead() }) index=TableIndexSpec(false) {
-            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());
-                alterExp.setUkName(index instanceof NamedConstraint
-                        ? ((NamedConstraint) index).getIndexName() : index.getName());
-                alterExp.setUk(index.getType().toUpperCase(Locale.ROOT).contains("KEY"));
-                alterExp.setUkTypeSpecified(index.getIndexKeyword() != null);
-                for (String option : new ArrayList<String>(index.getIndexSpec())) {
-                    if (option.toUpperCase(Locale.ROOT).startsWith("USING ")) {
-                        alterExp.addParameters("USING");
-                        alterExp.addParameters(option.substring("USING ".length()));
-                        index.getIndexSpec().remove(option);
-                    } else if (option.toUpperCase(Locale.ROOT).startsWith("COMMENT ")) {
-                        index.setCommentText(option.substring("COMMENT ".length()));
-                        index.getIndexSpec().remove(option);
-                    }
-                }
-            }
+            setAlterTableIndex(alterExp, index);
         }
         constraints=AlterExpressionConstraintState() { alterExp.setConstraints(constraints); }
         [ AlterExpressionUsingIndex(alterExp) ]
@@ -15485,21 +15517,10 @@ AlterExpression AlterExpressionAddAlterModify():
         |
         // Standalone FK now uses ForeignKeyIndex, same as CONSTRAINT FK
         (
-            { ForeignKeyIndex fkIndex; ReferentialAction ra; }
+            { ForeignKeyIndex fkIndex; }
             fkIndex = ForeignKeySpec(null)
             {
-                alterExp.setIndex(fkIndex);
-                // backward compat: populate deprecated FK fields from ForeignKeyIndex
-                alterExp.setFkColumns(fkIndex.getColumnsNames());
-                if (fkIndex.getTable() != null) {
-                    alterExp.setFkSourceSchema(fkIndex.getTable().getSchemaName());
-                    alterExp.setFkSourceTable(fkIndex.getTable().getName());
-                }
-                alterExp.setFkSourceColumns(fkIndex.getReferencedColumnNames());
-                ra = fkIndex.getReferentialAction(ReferentialAction.Type.DELETE);
-                if (ra != null) { alterExp.setReferentialAction(ra.getType(), ra.getAction()); }
-                ra = fkIndex.getReferentialAction(ReferentialAction.Type.UPDATE);
-                if (ra != null) { alterExp.setReferentialAction(ra.getType(), ra.getAction()); }
+                setAlterTableIndex(alterExp, fkIndex);
             }
         )
         |

src/test/java/net/sf/jsqlparser/statement/alter/PostgreSqlAlterConstraintTest.java

@@ -0,0 +1,104 @@
+/*-
+ * #%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.alter;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import net.sf.jsqlparser.JSQLParserException;
+import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect;
+import net.sf.jsqlparser.parser.CCJSqlParserUtil;
+import net.sf.jsqlparser.statement.create.table.CheckConstraint;
+import net.sf.jsqlparser.statement.create.table.ConstraintAttributes;
+import net.sf.jsqlparser.statement.create.table.ExcludeConstraint;
+import net.sf.jsqlparser.statement.create.table.Index;
+import net.sf.jsqlparser.util.TablesNamesFinder;
+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 PostgreSqlAlterConstraintTest {
+    @ParameterizedTest
+    @ValueSource(strings = {
+            "CONSTRAINT c CHECK (id > 0) NOT VALID",
+            "CHECK (id > 0) NOT VALID",
+            "CONSTRAINT c UNIQUE NULLS NOT DISTINCT (id) INCLUDE (other) WITH (fillfactor = 70) "
+                    + "USING INDEX TABLESPACE fast DEFERRABLE INITIALLY DEFERRED",
+            "UNIQUE (id) NOT DEFERRABLE INITIALLY IMMEDIATE",
+            "CONSTRAINT c PRIMARY KEY (id) DEFERRABLE INITIALLY DEFERRED",
+            "CONSTRAINT c FOREIGN KEY (id) REFERENCES parent(id) ON DELETE CASCADE "
+                    + "DEFERRABLE INITIALLY DEFERRED NOT VALID",
+            "FOREIGN KEY (id) REFERENCES parent(id) NOT VALID",
+            "CONSTRAINT c EXCLUDE USING gist ((id + 1) WITH =) WHERE (id > 0) DEFERRABLE",
+            "EXCLUDE USING gist (id WITH =)"
+    })
+    void sharedConstraintGrammarParsesAndRoundTrips(String definition)
+            throws JSQLParserException {
+        Alter alter = parse("ALTER TABLE t ADD " + definition);
+        StringBuilder buffer = new StringBuilder();
+        alter.accept(new StatementDeParser(buffer), null);
+        assertEquals(alter.toString(), buffer.toString());
+        assertEquals(alter.toString(), parse(buffer.toString()).toString());
+        assertEquals(1, alter.getAlterExpressions().size());
+    }
+
+    @Test
+    void constraintAttributesAndPredicatesRemainStructured() throws JSQLParserException {
+        Alter alter = parse("ALTER TABLE t ADD CONSTRAINT c CHECK (id > 0) NOT VALID, "
+                + "ADD CONSTRAINT u UNIQUE (id) DEFERRABLE INITIALLY DEFERRED");
+        CheckConstraint check = assertInstanceOf(CheckConstraint.class,
+                alter.getAlterExpressions().get(0).getIndex());
+        assertTrue(check.getConstraintAttributes().isNotValid());
+        Index unique = alter.getAlterExpressions().get(1).getIndex();
+        assertEquals(ConstraintAttributes.Initially.DEFERRED,
+                unique.getConstraintAttributes().getInitially());
+        assertEquals(Boolean.TRUE, unique.getConstraintAttributes().getDeferrable());
+        ExcludeConstraint exclude = assertInstanceOf(ExcludeConstraint.class,
+                parse("ALTER TABLE t ADD EXCLUDE USING gist ((id + 1) WITH =) WHERE (id > 0)")
+                        .getAlterExpressions().get(0).getIndex());
+        assertEquals("id > 0", exclude.getExpression().toString());
+    }
+
+    @Test
+    void commonProjectionPreservesLegacyKeyAccessors() throws JSQLParserException {
+        AlterExpression primary = parse("ALTER TABLE t ADD PRIMARY KEY (id)")
+                .getAlterExpressions().get(0);
+        assertThat(primary.getPkColumns()).containsExactly("id");
+        AlterExpression foreign =
+                parse("ALTER TABLE t ADD FOREIGN KEY (id) REFERENCES app.parent(id)")
+                        .getAlterExpressions().get(0);
+        assertThat(foreign.getFkColumns()).containsExactly("id");
+        assertEquals("app", foreign.getFkSourceSchema());
+        assertEquals("parent", foreign.getFkSourceTable());
+        assertThat(new TablesNamesFinder().getTables(
+                parse("ALTER TABLE t ADD FOREIGN KEY (id) REFERENCES app.parent(id) NOT VALID")))
+                .containsExactlyInAnyOrder("t", "app.parent");
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {
+            "ALTER TABLE t ADD CONSTRAINT c CHECK (id > 0) NOT INVALID",
+            "ALTER TABLE t ADD UNIQUE (id) INITIALLY UNKNOWN",
+            "ALTER TABLE t ADD EXCLUDE USING gist (id)",
+            "ALTER TABLE t ADD CONSTRAINT c CHECK (id > 0), ADD"
+    })
+    void invalidConstraintTailsAreRejected(String sql) {
+        assertThrows(JSQLParserException.class, () -> parse(sql));
+    }
+
+    private static Alter parse(String sql) throws JSQLParserException {
+        return (Alter) CCJSqlParserUtil.parse(sql,
+                parser -> parser.withDialect(Dialect.POSTGRESQL));
+    }
+}