Skip to content

Commit cec9fb8

Browse files
minleejaeminleejae
authored
fix: reuse table constraints for ALTER TABLE ADD in every dialect (#2847)
* fix: reuse table constraints for ALTER TABLE ADD in every dialect ALTER TABLE ADD parsed table constraints with the shared CREATE TABLE production only for the MySQL and PostgreSQL dialects (#2612, #2654). Without one of them, the generic ALTER branches rejected forms that CREATE TABLE accepts in every dialect: an unnamed CHECK (c > 0), NOT VALID, DEFERRABLE INITIALLY DEFERRED, an unnamed FOREIGN KEY with attributes and EXCLUDE. Route an ADD to the shared production in every dialect when it uses a form that only that production accepts: an unnamed CHECK, FOREIGN KEY, EXCLUDE, or INITIALLY IMMEDIATE|DEFERRED, NOT VALID or ENFORCED outside a CHECK before the end of the action. Forms that already parsed keep their generic representation, accessors and output; the PostgreSQL-only UNIQUE USING INDEX and NOT NULL constraints still need that dialect. The scan lives in the package-private AlterConstraintLookahead, like RoutineBodyBoundary, because the generated CCJSqlParser class is close to the size JaCoCo can still instrument. Signed-off-by: minleejae <mmj9808@gmail.com> * fix: parse VALIDATE CONSTRAINT as a structured action in every dialect ALTER TABLE t VALIDATE CONSTRAINT ck was a structured RelationAlterAction only with the PostgreSQL dialect. Elsewhere the remaining text became an UNSPECIFIC expression, so the constraint name could not be read and the action could not follow an ADD that now parses in every dialect. VALIDATE CONSTRAINT starts no other ALTER TABLE action, so recognise it in every dialect. Its output is unchanged. Signed-off-by: minleejae <mmj9808@gmail.com> * refactor: keep the ALTER constraint lookahead in the grammar Move AlterConstraintLookahead from its own file into the grammar as a private nested class, as done for TokenNesting in #2849. Split needsSharedTableConstraint into a check for the forms only the shared production accepts and a scan for constraint attributes. Codacy's PMD reported an NPath complexity of 440 for the combined method; the largest part is now 14. The nested class keeps its branches out of CCJSqlParser.class. Behaviour does not change. Signed-off-by: minleejae <mmj9808@gmail.com> --------- Signed-off-by: minleejae <mmj9808@gmail.com>
1 parent 220d0ed commit cec9fb8

3 files changed

Lines changed: 229 additions & 2 deletions

File tree

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

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2544,6 +2544,85 @@ public class CCJSqlParser extends AbstractJSqlParser<CCJSqlParser> {
25442544
}
25452545
}
25462546

2547+
/**
2548+
* Decides whether an ALTER TABLE ADD table constraint needs the table-constraint production
2549+
* shared with CREATE TABLE in dialects that otherwise keep the generic ALTER representation. A
2550+
* nested class keeps its branches out of CCJSqlParser.class, whose constant pool is close to
2551+
* what JaCoCo can still instrument.
2552+
*/
2553+
private static final class AlterConstraintLookahead {
2554+
private AlterConstraintLookahead() {}
2555+
2556+
/**
2557+
* Whether the action starting at {@code first} is an unnamed CHECK, a FOREIGN KEY,
2558+
* EXCLUDE, or uses a constraint attribute (INITIALLY IMMEDIATE|DEFERRED, NOT VALID, or
2559+
* ENFORCED outside CHECK) before the end of the action.
2560+
*/
2561+
static boolean needsSharedTableConstraint(Token first, CCJSqlParserTokenManager source) {
2562+
Token second = next(first, source);
2563+
Token body = first.kind == K_CONSTRAINT ? next(second, source) : first;
2564+
if (isSharedOnlyConstraint(first, second, body, source)) {
2565+
return true;
2566+
}
2567+
return (first.kind == K_CONSTRAINT || first.kind == K_PRIMARY
2568+
|| first.kind == K_UNIQUE)
2569+
&& hasConstraintAttribute(first, body.kind == K_CHECK, source);
2570+
}
2571+
2572+
/** An unnamed CHECK, a FOREIGN KEY or an EXCLUDE constraint. */
2573+
private static boolean isSharedOnlyConstraint(Token first, Token second, Token body,
2574+
CCJSqlParserTokenManager source) {
2575+
return first.kind == K_CHECK && second.kind == OPENING_BRACKET
2576+
|| first.kind == K_FOREIGN || isExcludeConstraint(body, source);
2577+
}
2578+
2579+
/** Whether a constraint attribute follows at the top level before the end of the action. */
2580+
private static boolean hasConstraintAttribute(Token first, boolean check,
2581+
CCJSqlParserTokenManager source) {
2582+
int depth = 0;
2583+
for (Token token = first; token.kind != EOF; token = next(token, source)) {
2584+
if (token.kind == OPENING_BRACKET) {
2585+
depth++;
2586+
} else if (token.kind == CLOSING_BRACKET) {
2587+
if (depth-- == 0) {
2588+
return false;
2589+
}
2590+
} else if (depth == 0) {
2591+
if (token.kind == ST_SEMICOLON || token.kind == K_COMMA) {
2592+
return false;
2593+
}
2594+
if (isConstraintAttribute(token, next(token, source), check)) {
2595+
return true;
2596+
}
2597+
}
2598+
}
2599+
return false;
2600+
}
2601+
2602+
private static boolean isConstraintAttribute(Token token, Token next, boolean check) {
2603+
if ("INITIALLY".equalsIgnoreCase(token.image)) {
2604+
return "IMMEDIATE".equalsIgnoreCase(next.image)
2605+
|| "DEFERRED".equalsIgnoreCase(next.image);
2606+
}
2607+
return token.kind == K_NOT && "VALID".equalsIgnoreCase(next.image)
2608+
|| token.kind == K_ENFORCED && !check;
2609+
}
2610+
2611+
/** EXCLUDE starts an exclusion constraint only before USING, its elements or WHERE. */
2612+
private static boolean isExcludeConstraint(Token token, CCJSqlParserTokenManager source) {
2613+
int following = next(token, source).kind;
2614+
return token.kind == K_EXCLUDE && (following == K_USING
2615+
|| following == OPENING_BRACKET || following == K_WHERE);
2616+
}
2617+
2618+
private static Token next(Token token, CCJSqlParserTokenManager source) {
2619+
if (token.next == null) {
2620+
token.next = source.getNextToken();
2621+
}
2622+
return token.next;
2623+
}
2624+
}
2625+
25472626
/**
25482627
* True when an IS after a condition opens a postfix test
25492628
* (IS [NOT] TRUE/FALSE/UNKNOWN/NULL/JSON) rather than IS DISTINCT FROM or
@@ -3064,6 +3143,8 @@ public class CCJSqlParser extends AbstractJSqlParser<CCJSqlParser> {
30643143
&& "EXPRESSION".equalsIgnoreCase(getToken(columnActionOffset + 1).image)) {
30653144
return true;
30663145
}
3146+
// VALIDATE CONSTRAINT starts no other ALTER TABLE action in any dialect.
3147+
if (first.kind == K_VALIDATE && getToken(2).kind == K_CONSTRAINT) { return true; }
30673148
if (!Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect))) { return false; }
30683149
if (first.kind == K_VALIDATE || first.kind == K_RESET || isKeywordAhead("INHERIT")
30693150
|| isKeywordAhead("REPLICA") || isKeywordAhead("OWNER") || isKeywordAhead("CLUSTER")
@@ -21081,13 +21162,14 @@ AlterExpression AlterExpressionAddAlterModify():
2108121162
constraint=PostgreSqlConstraintUsingIndex() { alterExp.setConstraint(constraint); }
2108221163
|
2108321164
LOOKAHEAD({ alterExp.getOperation() == AlterOperation.ADD
21084-
&& (Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect))
21165+
&& ((Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect))
2108521166
|| Dialect.MYSQL.name().equals(getAsString(Feature.dialect)))
2108621167
&& (getToken(1).kind == K_CONSTRAINT || getToken(1).kind == K_PRIMARY
2108721168
|| getToken(1).kind == K_UNIQUE || getToken(1).kind == K_FOREIGN
2108821169
|| getToken(1).kind == K_CHECK || getToken(1).kind == K_EXCLUDE
2108921170
|| getToken(1).kind == K_NOT
21090-
&& Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect))) })
21171+
&& Dialect.POSTGRESQL.name().equals(getAsString(Feature.dialect)))
21172+
|| AlterConstraintLookahead.needsSharedTableConstraint(getToken(1), token_source)) })
2109121173
element=TableConstraint(false) { setAlterTableElement(alterExp, element); }
2109221174
|
2109321175
LOOKAHEAD({ isTableIndexAhead() }) element=TableIndexSpec(false) {

‎src/test/java/net/sf/jsqlparser/statement/alter/AlterAddTableConstraintTest.java‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/*-
2+
* #%L
3+
* JSQLParser library
4+
* %%
5+
* Copyright (C) 2004 - 2026 JSQLParser
6+
* %%
7+
* Dual licensed under GNU LGPL 2.1 or Apache License 2.0
8+
* #L%
9+
*/
10+
package net.sf.jsqlparser.statement.alter;
11+
12+
import static net.sf.jsqlparser.test.TestUtils.assertSqlCanBeParsedAndDeparsed;
13+
import static org.assertj.core.api.Assertions.assertThat;
14+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
15+
import static org.junit.jupiter.api.Assertions.assertEquals;
16+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
17+
import static org.junit.jupiter.api.Assertions.assertNull;
18+
import static org.junit.jupiter.api.Assertions.assertThrows;
19+
import static org.junit.jupiter.api.Assertions.assertTrue;
20+
21+
import java.util.Arrays;
22+
import java.util.List;
23+
import java.util.function.Consumer;
24+
import net.sf.jsqlparser.JSQLParserException;
25+
import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect;
26+
import net.sf.jsqlparser.parser.CCJSqlParser;
27+
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
28+
import net.sf.jsqlparser.statement.create.table.CheckConstraint;
29+
import net.sf.jsqlparser.statement.create.table.ConstraintAttributes;
30+
import net.sf.jsqlparser.statement.create.table.CreateTable;
31+
import net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint;
32+
import net.sf.jsqlparser.statement.create.table.NamedConstraint;
33+
import net.sf.jsqlparser.util.TablesNamesFinder;
34+
import org.junit.jupiter.api.Test;
35+
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.ValueSource;
37+
38+
/**
39+
* ALTER TABLE ADD accepts the table constraint forms of CREATE TABLE in every dialect, not only
40+
* with the MySQL and PostgreSQL dialects.
41+
*/
42+
class AlterAddTableConstraintTest {
43+
private static final List<Dialect> DIALECTS =
44+
Arrays.asList(null, Dialect.ANSI_SQL, Dialect.ORACLE, Dialect.SQLSERVER, Dialect.H2);
45+
46+
@ParameterizedTest
47+
@ValueSource(strings = {
48+
"CHECK (id > 0)",
49+
"CHECK (id > 0) NOT VALID",
50+
"CONSTRAINT ck CHECK (id > 0) NOT VALID",
51+
"FOREIGN KEY (id) REFERENCES parent (id) DEFERRABLE INITIALLY DEFERRED",
52+
"CONSTRAINT fk FOREIGN KEY (id) REFERENCES parent (id) NOT VALID",
53+
"CONSTRAINT pk PRIMARY KEY (id) DEFERRABLE INITIALLY DEFERRED",
54+
"CONSTRAINT uq UNIQUE (id) DEFERRABLE INITIALLY IMMEDIATE",
55+
"CONSTRAINT ex EXCLUDE USING gist (id WITH =)"})
56+
void createAndAlterExposeTheSameConstraintInEveryDialect(String definition)
57+
throws JSQLParserException {
58+
for (Dialect dialect : DIALECTS) {
59+
Consumer<CCJSqlParser> configuration = parser -> {
60+
if (dialect != null) {
61+
parser.withDialect(dialect);
62+
}
63+
};
64+
CreateTable create = (CreateTable) CCJSqlParserUtil
65+
.parse("CREATE TABLE t (id INT, " + definition + ")", configuration);
66+
Alter alter = (Alter) assertSqlCanBeParsedAndDeparsed(
67+
"ALTER TABLE t ADD " + definition + ", ADD COLUMN other INT", true,
68+
configuration);
69+
NamedConstraint expected = create.getTableConstraints().get(0);
70+
NamedConstraint actual = alter.getAlterExpressions().get(0).getConstraint();
71+
assertEquals(expected.getClass(), actual.getClass(), String.valueOf(dialect));
72+
assertEquals(expected.toString(), actual.toString(), String.valueOf(dialect));
73+
assertEquals(2, alter.getAlterExpressions().size());
74+
}
75+
}
76+
77+
@Test
78+
void attributesAndReferencesAreStructuredWithoutADialect() throws JSQLParserException {
79+
String sql = "ALTER TABLE t ADD CONSTRAINT fk FOREIGN KEY (id) REFERENCES parent (id) "
80+
+ "DEFERRABLE INITIALLY DEFERRED";
81+
Alter alter = (Alter) CCJSqlParserUtil.parse(sql);
82+
ForeignKeyConstraint foreignKey = assertInstanceOf(ForeignKeyConstraint.class,
83+
alter.getAlterExpressions().get(0).getConstraint());
84+
ConstraintAttributes attributes = foreignKey.getConstraintAttributes();
85+
assertEquals(Boolean.TRUE, attributes.getDeferrable());
86+
assertEquals(ConstraintAttributes.Initially.DEFERRED, attributes.getInitially());
87+
assertThat(TablesNamesFinder.findTables(sql)).containsExactlyInAnyOrder("t", "parent");
88+
89+
CheckConstraint check = assertInstanceOf(CheckConstraint.class,
90+
((Alter) CCJSqlParserUtil.parse("ALTER TABLE t ADD CHECK (id > 0) NOT VALID"))
91+
.getAlterExpressions().get(0).getConstraint());
92+
assertNull(check.getName());
93+
assertTrue(check.getConstraintAttributes().isNotValid());
94+
}
95+
96+
@Test
97+
void formsThatAlreadyParsedKeepTheGenericRepresentation() throws JSQLParserException {
98+
AlterExpression deferrable = ((Alter) CCJSqlParserUtil
99+
.parse("ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (id) DEFERRABLE"))
100+
.getAlterExpressions().get(0);
101+
assertThat(deferrable.getConstraints()).hasSize(1);
102+
assertInstanceOf(DeferrableConstraint.class, deferrable.getConstraints().get(0));
103+
104+
AlterExpression unique = ((Alter) CCJSqlParserUtil
105+
.parse("ALTER TABLE t ADD CONSTRAINT UNIQUE KEY uk (id)"))
106+
.getAlterExpressions().get(0);
107+
assertEquals("UNIQUE KEY", unique.getConstraintType());
108+
}
109+
110+
@ParameterizedTest
111+
@ValueSource(strings = {"ALTER TABLE t ADD CONSTRAINT nn NOT NULL id",
112+
"ALTER TABLE t ADD CONSTRAINT uk UNIQUE USING INDEX t_idx"})
113+
void postgreSqlOnlyFormsStillRequireThePostgreSqlDialect(String sql) {
114+
assertThrows(JSQLParserException.class, () -> CCJSqlParserUtil.parse(sql));
115+
assertDoesNotThrow(
116+
() -> CCJSqlParserUtil.parse(sql,
117+
parser -> parser.withDialect(Dialect.POSTGRESQL)));
118+
}
119+
120+
@Test
121+
void enforcedPrimaryKeyIsRejected() {
122+
JSQLParserException exception = assertThrows(JSQLParserException.class,
123+
() -> CCJSqlParserUtil
124+
.parse("ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (id) ENFORCED"));
125+
assertThat(exception).hasStackTraceContaining(
126+
"ENFORCED is supported only for CHECK and foreign key constraints");
127+
}
128+
}

‎src/test/java/net/sf/jsqlparser/statement/alter/PostgreSqlRelationAlterTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,23 @@ void bulkTablespaceMovesShareOneModel(String objectType) throws JSQLParserExcept
229229
assertEquals(2, CCJSqlParserUtil.parseStatements(statement + "; SELECT 1").size());
230230
}
231231

232+
@Test
233+
void validateConstraintIsStructuredWithoutThePostgreSqlDialect() throws JSQLParserException {
234+
for (Dialect dialect : new Dialect[] {null, Dialect.ANSI_SQL, Dialect.MYSQL}) {
235+
Alter alter = (Alter) assertSqlCanBeParsedAndDeparsed(
236+
"ALTER TABLE t ADD CHECK (c > 0) NOT VALID, VALIDATE CONSTRAINT ck", true,
237+
p -> {
238+
if (dialect != null) {
239+
p.withDialect(dialect);
240+
}
241+
});
242+
RelationAlterAction validate = assertInstanceOf(RelationAlterAction.class,
243+
alter.getAlterExpressions().get(1));
244+
assertEquals(RelationAlterAction.Kind.VALIDATE_CONSTRAINT, validate.getKind());
245+
assertEquals("ck", validate.getConstraintName());
246+
}
247+
}
248+
232249
private static Statement parse(String sql) throws JSQLParserException {
233250
return CCJSqlParserUtil.parse(sql, p -> p.withDialect(Dialect.POSTGRESQL));
234251
}

0 commit comments

Comments
 (0)