From ee4f58ffe9ace9dccca2be165a98d13af83ed148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20=C3=87engel?= Date: Sat, 10 Oct 2026 17:06:03 +0300 Subject: [PATCH] feat(schema): ignore column-neutral ALTER TABLE actions and EXCLUDE ALTER TABLE actions that cannot change a column's existence, name, type, or nullability are now ignored: SET/DROP DEFAULT, SET STATISTICS, STORAGE, and COMPRESSION, DROP EXPRESSION, identity changes, row-level security switches, partition attachment, OWNER TO, trigger and rule switches, VALIDATE CONSTRAINT, REPLICA IDENTITY, clustering, storage parameters, tablespace, logging, access method, inheritance, and OF. Actions the parser reports only as trailing text are split at top-level commas and each one must be such an action, so a modeled action written after one is rejected instead of skipped. Any other action is rejected as "Unsupported ALTER TABLE action: at line ". CREATE TABLE now accepts EXCLUDE table constraints without recording them. --- docs/postgresql.md | 109 ++++++--- .../schema/parser/DefaultSchemaParser.java | 203 ++++++++++++++-- .../schema/parser/SchemaSourceSplitter.java | 26 +++ .../parser/DefaultSchemaParserTest.java | 217 +++++++++++++++--- 4 files changed, 472 insertions(+), 83 deletions(-) diff --git a/docs/postgresql.md b/docs/postgresql.md index 06dafdd..dbbaff9 100644 --- a/docs/postgresql.md +++ b/docs/postgresql.md @@ -362,6 +362,7 @@ are recorded with their declared type; see | Column-level `CHECK (...)` | Accepted and ignored. | | Table-level `FOREIGN KEY (...) REFERENCES ...`, named or unnamed | Accepted and ignored. | | Table-level `CHECK (...)`, named or unnamed | Accepted and ignored. | +| Table-level `EXCLUDE USING ... (...)`, named or unnamed | Accepted and ignored. | | A table statement sqlcj's parser cannot read, such as `ALTER FOREIGN TABLE` | Rejected; see [Ignored Statements](#ignored-statements). | | Any other table-constraint kind | Rejected. | | Unparsable SQL | Rejected. | @@ -395,7 +396,7 @@ statements and files before them left; the enum-type statements are listed in | `ALTER TABLE ... ALTER COLUMN ... SET NOT NULL` | Models the column non-null. | | `ALTER TABLE ... ALTER COLUMN ... DROP NOT NULL` | Models the column nullable. | | `ALTER TABLE IF EXISTS ...` | Does nothing when the table is not modeled. It covers only the table, so a missing column of a modeled table still fails. | -| Any other `ALTER TABLE` action outside the constraint actions listed in [Ignored Statements](#ignored-statements), such as `SET DEFAULT` | Rejected as `Unsupported ALTER TABLE action at line `. | +| Any other `ALTER TABLE` action outside the ignored actions listed in [Ignored Statements](#ignored-statements), such as `SET SCHEMA` | Rejected as `Unsupported ALTER TABLE action: at line `, quoting the action's own SQL. | | `DROP` of any object other than a table, such as `DROP VIEW` or `DROP INDEX` | Ignored; see [Ignored Statements](#ignored-statements). | One `ALTER TABLE` may state several actions; they are applied in the written @@ -462,7 +463,7 @@ what they would do to the schema model: | Statement | Diagnostic | | --- | --- | -| An `ALTER TABLE` action outside [Ordered Table DDL](#ordered-table-ddl) and the constraint actions below | `Unsupported ALTER TABLE action at line ` | +| An `ALTER TABLE` action outside [Ordered Table DDL](#ordered-table-ddl) and the ignored actions below | `Unsupported ALTER TABLE action: at line `, quoting the action's own SQL | | An `ALTER TYPE` action other than `ADD VALUE` | `Unsupported ALTER TYPE action at line ` | | A statement sqlcj's parser cannot read that opens as table or type DDL, outside the `ALTER TABLE` actions below | The parser's own reason with the line and column of the unexpected token, as in `Encountered unexpected token: "DATA" at line 18, column 41` | | A statement sqlcj's parser reports only as opaque text, or as several statements, and that opens as table or type DDL | `Unsupported schema statement: ALTER FOREIGN TABLE at line `, quoting the opening words | @@ -475,27 +476,60 @@ in upper case, single-spaced, so both `ALTER FOREIGN TABLE ft ADD COLUMN x integer` and its lower-case spelling are rejected as `Unsupported schema statement: ALTER FOREIGN TABLE at line `. -These `ALTER TABLE` actions are ignored like any other unmodeled statement, -except that the statement still resolves its table: +Every `ALTER TABLE` action that cannot change a column's existence, name, type, +or nullability is ignored like any other unmodeled statement, except that the +statement still resolves its table: | Action | Notes | | --- | --- | -| `ALTER TABLE ... ADD [CONSTRAINT name] PRIMARY KEY (...)` | Named or unnamed. | -| `ALTER TABLE ... ADD [CONSTRAINT name] UNIQUE (...)` | Named or unnamed. | -| `ALTER TABLE ... ADD [CONSTRAINT name] FOREIGN KEY (...) REFERENCES ...` | Named or unnamed. | -| `ALTER TABLE ... ADD CONSTRAINT name CHECK (...)` | | -| `ALTER TABLE ... DROP CONSTRAINT [IF EXISTS] name` | | -| `ALTER TABLE ... RENAME CONSTRAINT` | | +| `ADD [CONSTRAINT name] PRIMARY KEY (...)` | Named or unnamed. | +| `ADD [CONSTRAINT name] UNIQUE (...)` | Named or unnamed. | +| `ADD [CONSTRAINT name] FOREIGN KEY (...) REFERENCES ...` | Named or unnamed. | +| `ADD CONSTRAINT name CHECK (...)` | | +| `DROP CONSTRAINT [IF EXISTS] name` | | +| `RENAME CONSTRAINT` | | +| `ALTER [COLUMN] c SET DEFAULT ...` and `DROP DEFAULT` | A `DEFAULT` is accepted and ignored in a `CREATE TABLE` as well. | +| `ALTER [COLUMN] c SET STATISTICS`, `SET STORAGE`, and `SET COMPRESSION` | Column storage, not the column's type. | +| `ALTER [COLUMN] c DROP EXPRESSION` | Only the readable spelling; `DROP EXPRESSION IF EXISTS` is unreadable. | +| `ALTER [COLUMN] c ADD GENERATED ... AS IDENTITY`, `SET GENERATED ...`, a sequence option such as `SET INCREMENT BY 2`, `RESTART`, and `DROP IDENTITY [IF EXISTS]` | PostgreSQL requires the column to be `NOT NULL` already, so an identity never changes nullability. | +| `ENABLE`, `DISABLE`, `FORCE`, and `NO FORCE ROW LEVEL SECURITY` | | +| `ATTACH PARTITION` and `DETACH PARTITION` | The partition's own table is modeled by its own statements. | +| `OWNER TO` | | +| `ENABLE TRIGGER`, `DISABLE TRIGGER`, `ENABLE REPLICA TRIGGER`, and `ENABLE ALWAYS TRIGGER`, and the same four for `RULE` | | +| `VALIDATE CONSTRAINT` | | +| `REPLICA IDENTITY ...` | | +| `CLUSTER ON` and `SET WITHOUT CLUSTER` | | +| `SET WITHOUT OIDS` | | +| `SET (...)` and `RESET (...)` | Storage parameters. | +| `SET TABLESPACE`, `SET LOGGED`, `SET UNLOGGED`, and `SET ACCESS METHOD` | | +| `INHERIT` and `NO INHERIT` | A table's columns are modeled from its own statements; inherited columns are not modeled. | +| `OF type` and `NOT OF` | | + +An ignored action resolves its table but not its column, so +`ALTER TABLE users ALTER COLUMN nickname SET STATISTICS 100` is accepted for a +column that is not modeled, while `ALTER TABLE payments OWNER TO app` fails as +`Table not found in schema: payments`, exactly as any other `ALTER TABLE` of a +missing table does, and `ALTER TABLE IF EXISTS payments OWNER TO app` does +nothing. A constraint an `ALTER TABLE` adds is not recorded, so it is not carried into -the schema model the way a `CREATE TABLE` constraint is. Because the table is -still resolved, -`ALTER TABLE payments ADD CONSTRAINT payments_pkey PRIMARY KEY (id)` fails when -`payments` is not modeled, exactly as any other `ALTER TABLE` of a missing -table does. Such an action may stand alone or beside modeled actions of one -`ALTER TABLE`, so -`ALTER TABLE users ADD COLUMN age INTEGER, ADD CONSTRAINT users_age_check CHECK (age > 0)` -appends `age` and records nothing for the constraint. +the schema model the way a `CREATE TABLE` constraint is. An ignored action may +stand alone or beside modeled actions of one `ALTER TABLE`, which are applied in +the written order, so +`ALTER TABLE users ADD COLUMN age INTEGER, ALTER COLUMN name SET DEFAULT 'new', OWNER TO app` +appends `age` and records nothing else. + +sqlcj's parser has no form of its own for the table-level actions listed from +`OWNER TO` down in the table above. It reports such an action as the text that +runs to the end of the statement, so sqlcj splits that text at the commas +outside parentheses, with its own SQL lexer, and every action in it must be one +of those. `ALTER TABLE users SET SCHEMA archive` is therefore rejected as +`Unsupported ALTER TABLE action: SET SCHEMA archive at line `, because it +moves the table out of the one namespace sqlcj models, and a modeled action +written after such an action is rejected rather than skipped, so +`ALTER TABLE users OWNER TO app, ADD COLUMN x int` is rejected as +`Unsupported ALTER TABLE action: ADD COLUMN x int at line `. Written the +other way round, `ALTER TABLE users ADD COLUMN x int, OWNER TO app` appends `x`. An `ALTER TABLE` sqlcj's parser cannot read is classified by its words instead, and ignored when every one of its top-level actions — the parts separated by @@ -654,7 +688,7 @@ and recorded type: ```text sqlcj: Invalid schema source /home/dev/project/schema.sql: Unsupported schema statement: ALTER FOREIGN TABLE at line 12 -sqlcj: Invalid schema source /home/dev/project/schema.sql: Unsupported ALTER TABLE action at line 18 +sqlcj: Invalid schema source /home/dev/project/schema.sql: Unsupported ALTER TABLE action: SET SCHEMA archive at line 18 sqlcj: Invalid schema source /home/dev/project/schema.sql: Encountered unexpected token: ";" at line 4, column 1 sqlcj: Invalid query 'ListTags' in /home/dev/project/queries.sql at line 5: Column 'tags' has unsupported type JSONB[] ``` @@ -803,9 +837,11 @@ Enum types: `DefaultSchemaParserTest.shouldParseTableWithoutConstraints`, `DefaultSchemaParserTest.shouldCanonicalizeQuotedTableAndColumnNames`, `DefaultSchemaParserTest.shouldParseColumnSpecificationsThatDoNotAffectTypes`, - `DefaultSchemaParserTest.shouldIgnoreForeignKeyAndCheckTableConstraints`, and + `DefaultSchemaParserTest.shouldIgnoreForeignKeyAndCheckTableConstraints`, + `DefaultSchemaParserTest.shouldIgnoreExcludeTableConstraints`, and `DefaultSchemaParserTest.shouldParseNamedTableConstraintsAlongsideIgnoredOnes` - cover the modeled and ignored constructs. + cover the modeled and ignored constructs, including a named and an unnamed + `EXCLUDE` beside a recorded `PRIMARY KEY`. - `DefaultSchemaParserTest.shouldRejectUnsupportedSchemaStatement` and `DefaultSchemaParserTest.shouldThrowSchemaParseExceptionForInvalidSql` cover the rejected input. @@ -837,11 +873,9 @@ Ordered table DDL: - `DefaultSchemaParserTest.shouldReportTheMissingTableOfAStatement`, `DefaultSchemaParserTest.shouldReportTheMissingColumnOfAnAlterTableAction`, `DefaultSchemaParserTest.shouldReportAStatementThatRepeatsATableName`, - `DefaultSchemaParserTest.shouldReportAStatementThatRepeatsAColumnName`, - `DefaultSchemaParserTest.shouldRejectUnsupportedAlterTableAction`, - `DefaultSchemaParserTest.shouldRejectAlterColumnSetStatistics`, and - `DefaultSchemaParserTest.shouldRejectAlterColumnAddIdentity` cover the - rejected statements and their messages. + `DefaultSchemaParserTest.shouldReportAStatementThatRepeatsAColumnName`, and + `DefaultSchemaParserTest.shouldReportTheLineOfARejectedAlterTableAction` + cover the rejected statements and their messages. - `SqlcjCompilerIntegrationTest.shouldGenerateTheSameRepositoryFromAlteringMigrationsAndASnapshot` generates one compilable repository from migrations that rename, add, drop, and alter columns, byte-identical to the one generated from the equivalent @@ -860,13 +894,24 @@ Ignored statements: `ALTER FUNCTION`, `ALTER SCHEMA`, `CREATE AGGREGATE`, and `CREATE CAST`; and `DefaultSchemaParserTest.shouldIgnoreConstraintAlterTableActions` covers the named and unnamed constraint actions. +- `DefaultSchemaParserTest.shouldIgnoreTheAlterTableActionsThatCannotChangeAColumn` + covers one spelling per ignored `ALTER TABLE` action, including the lower-case + and `ALTER TABLE ONLY public.users` spellings and two actions classified by + their words in one statement. - `DefaultSchemaParserTest.shouldNotResolveAnIgnoredStatementAgainstTheSchema` covers that an ignored statement, including a view over an unmodeled table and - a `TRUNCATE` of one, may name an unmodeled table or column, and + a `TRUNCATE` of one, may name an unmodeled table or column; `DefaultSchemaParserTest.shouldReportTheMissingTableOfAnIgnoredConstraintAction` - covers that an ignored `ALTER TABLE` action still resolves its table. -- `DefaultSchemaParserTest.shouldApplyAModeledActionBesideAnIgnoredConstraintAction` - covers an ignored action beside a modeled one. + and + `DefaultSchemaParserTest.shouldReportTheMissingTableOfAnIgnoredAction` + cover that an ignored `ALTER TABLE` action still resolves its table and that + `IF EXISTS` skips it; and + `DefaultSchemaParserTest.shouldIgnoreAnAlterColumnActionOnAMissingColumn` + covers that it does not resolve its column. +- `DefaultSchemaParserTest.shouldApplyAModeledActionBesideAnIgnoredConstraintAction`, + `DefaultSchemaParserTest.shouldApplyAnAddedColumnBesideIgnoredActions`, and + `DefaultSchemaParserTest.shouldApplyAnAddedColumnAfterARowLevelSecurityAction` + cover an ignored action beside a modeled one, in both written orders. - `DefaultSchemaParserTest.shouldIgnoreEveryUnreadableStatementThatIsNotTableOrTypeDdl` covers one spelling per statement kind sqlcj's parser cannot read, including the untagged, tagged, and `LANGUAGE` forms of `DO`, `DROP TYPE`, every @@ -881,7 +926,9 @@ Ignored statements: the schema does not model, including the multi-line `pg_dump` identity form. - `DefaultSchemaParserTest.shouldRejectUnsupportedStatementsInSqlTerms` covers the exact message and line of each statement and action rejected in SQL terms, - including the upper- and lower-case `ALTER FOREIGN TABLE`, and + including `SET SCHEMA` alone and after a modeled action, a modeled action + written after an ignored one, an unrecognized `ALTER COLUMN` action, and the + upper- and lower-case `ALTER FOREIGN TABLE`, and `DefaultSchemaParserTest.shouldRejectUnsupportedSchemaStatement`, `DefaultSchemaParserTest.shouldReportTheLineOfARejectedStatementAfterCommentsAndAFunctionBody`, and diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/DefaultSchemaParser.java b/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/DefaultSchemaParser.java index e048fcc..7e38f6b 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/DefaultSchemaParser.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/DefaultSchemaParser.java @@ -21,6 +21,7 @@ import net.sf.jsqlparser.statement.create.table.CheckConstraint; import net.sf.jsqlparser.statement.create.table.ColumnDefinition; import net.sf.jsqlparser.statement.create.table.CreateTable; +import net.sf.jsqlparser.statement.create.table.ExcludeConstraint; import net.sf.jsqlparser.statement.create.table.ForeignKeyIndex; import net.sf.jsqlparser.statement.create.table.Index; import net.sf.jsqlparser.statement.create.type.CreateType; @@ -268,10 +269,11 @@ private boolean addsIgnoredConstraint(List action) { /** * Reports whether the tokens from {@code index} on are {@code words}, - * compared case-insensitively. + * compared case-insensitively. A list the parser did not state at all states + * no words. */ private boolean statesWords(List tokens, int index, String... words) { - if (index + words.length > tokens.size()) { + if (tokens == null || index + words.length > tokens.size()) { return false; } @@ -519,10 +521,12 @@ private void applyAlterTable(Alter alter, int line, List enums, ListAn action that cannot change a modeled column's existence, name, type, + * or nullability is accepted and leaves the table unchanged. An action that + * would change a column in a way sqlcj does not model, and an action sqlcj + * does not recognize, is rejected naming the action's own SQL. */ private Table applyAlterExpression( int line, @@ -554,11 +558,105 @@ private Table applyAlterExpression( return alterColumns(line, expression, enums, table); } - if (isIgnoredConstraintAction(expression)) { + if (isIgnoredConstraintAction(expression) || isIgnoredTableAction(operation)) { return table; } - throw unsupportedAlterTableAction(line); + if (operation == AlterOperation.UNSPECIFIC) { + return ignoreUnspecificActions(line, expression, table); + } + + throw unsupportedAlterTableAction(line, expression); + } + + /** + * Reports whether one {@code ALTER TABLE} action is a table-level action the + * parser has a form of its own for that cannot change a column: the + * row-level-security switches and attaching or detaching a partition. + */ + private boolean isIgnoredTableAction(AlterOperation operation) { + return switch (operation) { + case ENABLE_ROW_LEVEL_SECURITY, + DISABLE_ROW_LEVEL_SECURITY, + FORCE_ROW_LEVEL_SECURITY, + NO_FORCE_ROW_LEVEL_SECURITY, + ATTACH_PARTITION, + DETACH_PARTITION -> true; + default -> false; + }; + } + + /** + * Accepts the actions the parser has no form of its own for, which it + * reports as one piece of text running to the end of the statement, so + * several written actions may stand in it. The text is lexed with the + * parser's own lexer and split at the commas outside parentheses, and each + * of its actions must begin as a table-level action that cannot change a + * column. The first action that does not is rejected, so a modeled action + * written after such an action is rejected rather than silently skipped. + */ + private Table ignoreUnspecificActions(int line, AlterExpression expression, Table table) { + String specifier = expression.getOptionalSpecifier(); + + if (specifier == null) { + throw unsupportedAlterTableAction(line, expression); + } + + for (List action : topLevelActions(SchemaSourceSplitter.tokens(specifier))) { + if (!ignoresTableLevelAction(action)) { + throw unsupportedAlterTableAction(line, String.join(" ", action)); + } + } + + return table; + } + + /** + * Reports whether one action of the unspecific text begins as a + * PostgreSQL 16 table-level action that cannot change a column: ownership, + * trigger and rule switches, constraint validation, replica identity, + * clustering, storage parameters, the tablespace, logging, the access + * method, inheritance, and the typed-table form. {@code SET SCHEMA} is not + * among them, because it moves the table out of the namespace sqlcj models. + */ + private boolean ignoresTableLevelAction(List action) { + return statesWords(action, 0, "OWNER", "TO") + || switchesTriggerOrRule(action) + || statesWords(action, 0, "VALIDATE", "CONSTRAINT") + || statesWords(action, 0, "REPLICA", "IDENTITY") + || statesWords(action, 0, "CLUSTER", "ON") + || statesWords(action, 0, "SET", "WITHOUT", "CLUSTER") + || statesWords(action, 0, "SET", "WITHOUT", "OIDS") + || statesWords(action, 0, "SET", "(") + || statesWords(action, 0, "RESET", "(") + || statesWords(action, 0, "SET", "TABLESPACE") + || statesWords(action, 0, "SET", "LOGGED") + || statesWords(action, 0, "SET", "UNLOGGED") + || statesWords(action, 0, "SET", "ACCESS", "METHOD") + || statesWords(action, 0, "INHERIT") + || statesWords(action, 0, "NO", "INHERIT") + || statesWords(action, 0, "OF") + || statesWords(action, 0, "NOT", "OF"); + } + + /** + * Reports whether one action switches a trigger or a rule: + * {@code ENABLE} or {@code DISABLE}, and {@code ENABLE REPLICA} or + * {@code ENABLE ALWAYS}, of either object. + */ + private boolean switchesTriggerOrRule(List action) { + for (String object : List.of("TRIGGER", "RULE")) { + if ( + statesWords(action, 0, "ENABLE", object) + || statesWords(action, 0, "DISABLE", object) + || statesWords(action, 0, "ENABLE", "REPLICA", object) + || statesWords(action, 0, "ENABLE", "ALWAYS", object) + ) { + return true; + } + } + + return false; } /** @@ -597,7 +695,9 @@ private boolean isIgnoredConstraintKind(Index index) { /** * Applies one {@code ALTER COLUMN} action: a new type, which keeps the - * column's position and nullability, or a nullability change. + * column's position and nullability, or a nullability change. A column + * action that cannot change the column sqlcj models leaves the table + * unchanged, without its column being resolved. */ private Table alterColumns( int line, @@ -605,12 +705,20 @@ private Table alterColumns( List enums, Table table ) { + if (isNotEmpty(expression.getColumnSetDefaultList()) || isNotEmpty(expression.getColumnDropDefaultList())) { + return table; + } + if (isNotEmpty(expression.getColDataTypeList())) { - if (!statesNewTypes(expression.getColDataTypeList())) { - throw unsupportedAlterTableAction(line); + if (statesNewTypes(expression.getColDataTypeList())) { + return changeColumnTypes(expression, enums, table); + } + + if (ignoresColumnActions(expression.getColDataTypeList())) { + return table; } - return changeColumnTypes(expression, enums, table); + throw unsupportedAlterTableAction(line, expression); } if (isNotEmpty(expression.getColumnSetNotNullList())) { @@ -633,14 +741,14 @@ private Table alterColumns( ); } - throw unsupportedAlterTableAction(line); + throw unsupportedAlterTableAction(line, expression); } /** * Reports whether every entry of an {@code ALTER COLUMN} action states a new * type. The parser reports other column actions, such as * {@code SET STATISTICS} or an identity change, in the same list without a - * declared type, and those are not supported. + * declared type. */ private boolean statesNewTypes(List definitions) { return definitions.stream() @@ -649,6 +757,46 @@ private boolean statesNewTypes(List definitions) ); } + /** + * Reports whether every entry of an {@code ALTER COLUMN} action states a + * column action that cannot change the column sqlcj models. + */ + private boolean ignoresColumnActions(List definitions) { + return definitions.stream().allMatch(this::ignoresColumnAction); + } + + /** + * Reports whether one entry of an {@code ALTER COLUMN} action states an + * identity change, {@code SET STATISTICS}, {@code SET STORAGE}, + * {@code SET COMPRESSION}, or {@code DROP EXPRESSION}. None of them can + * change a column's existence, name, type, or nullability, which comes from + * {@code NOT NULL} alone; PostgreSQL requires a column to be + * {@code NOT NULL} already before {@code ADD GENERATED ... AS IDENTITY}. The + * parser reports each of them in the entry list of the action, without a + * declared type. + */ + private boolean ignoresColumnAction(AlterExpression.ColumnDataType definition) { + if (definition.isWithType()) { + return false; + } + + if (isNotEmpty(definition.getIdentityAlterations())) { + return true; + } + + if (definition.getColDataType() == null) { + return statesWords(definition.getColumnSpecs(), 0, "DROP", "EXPRESSION"); + } + + if (!"SET".equalsIgnoreCase(definition.getColDataType().getDataType())) { + return false; + } + + return statesWords(definition.getColumnSpecs(), 0, "STATISTICS") + || statesWords(definition.getColumnSpecs(), 0, "STORAGE") + || statesWords(definition.getColumnSpecs(), 0, "COMPRESSION"); + } + /** * Appends each added column, typed exactly as a {@code CREATE TABLE} column * of the same declaration, and records its column-level constraints. @@ -931,9 +1079,24 @@ private IllegalArgumentException labelAlreadyExists(String typeName, String labe ); } - private UnsupportedOperationException unsupportedAlterTableAction(int line) { + /** + * The failure of one rejected {@code ALTER TABLE} action, quoting the action + * as the parser writes it back, with its whitespace runs collapsed to single + * spaces. + */ + private UnsupportedOperationException unsupportedAlterTableAction( + int line, + AlterExpression expression + ) { + return unsupportedAlterTableAction( + line, + expression.toString().replaceAll("\\s+", " ").trim() + ); + } + + private UnsupportedOperationException unsupportedAlterTableAction(int line, String action) { return new UnsupportedOperationException( - "Unsupported ALTER TABLE action at line %d".formatted(line) + "Unsupported ALTER TABLE action: %s at line %d".formatted(action, line) ); } @@ -1095,11 +1258,13 @@ private List parseTableConstraints(CreateTable createTable) { } /** - * Foreign key and check constraints are accepted but not modeled, because - * they do not affect the generated Java types. + * Foreign key, check, and exclusion constraints are accepted but not + * modeled, because they do not affect the generated Java types. */ private boolean isIgnoredTableConstraint(Index index) { - return index instanceof ForeignKeyIndex || index instanceof CheckConstraint; + return index instanceof ForeignKeyIndex + || index instanceof CheckConstraint + || index instanceof ExcludeConstraint; } private ConstraintType constraintType(Index index) { diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaSourceSplitter.java b/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaSourceSplitter.java index 94aea1d..8a15eda 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaSourceSplitter.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaSourceSplitter.java @@ -123,6 +123,32 @@ static List split(String source) { return statements; } + /** + * The images of the tokens of one piece of SQL, in order, lexed with the + * same token manager {@link #split(String)} uses, so one SQL grammar reads + * every word sqlcj classifies. A lexical error fails as it does there. + */ + static List tokens(String sql) { + CCJSqlParserTokenManager tokenManager = tokenManager(sql, 1, 1); + List tokens = new ArrayList<>(); + + try { + while (true) { + Token token = tokenManager.getNextToken(); + + if (token.kind == CCJSqlParserConstants.EOF) { + break; + } + + tokens.add(token.image); + } + } catch (TokenMgrException e) { + throw new SchemaParseException(SqlParseReason.of(e, true), e); + } + + return tokens; + } + /** * A token manager over {@code text}, which begins at {@code line} and * {@code column} of the whole source, so every token it reports carries a diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/schema/parser/DefaultSchemaParserTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/schema/parser/DefaultSchemaParserTest.java index 4117989..0c2a904 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/schema/parser/DefaultSchemaParserTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/schema/parser/DefaultSchemaParserTest.java @@ -797,62 +797,180 @@ CREATE TABLE users ( } /** - * An {@code ALTER TABLE} action outside the supported forms, such as - * {@code SET DEFAULT}, is rejected like any other unsupported statement. + * Every parsed {@code ALTER TABLE} action that cannot change a column's + * existence, name, type, or nullability is accepted and leaves the tables + * the statements before it left, whether the parser has a form of its own + * for the action or reports it only as unspecific text. */ - @Test - void shouldRejectUnsupportedAlterTableAction() { - Schema schema = parser.parse(BASE_SCHEMA); + @ParameterizedTest + @ValueSource( + strings = { + "ALTER TABLE users ALTER COLUMN name SET DEFAULT 'new';", + "ALTER TABLE users ALTER COLUMN name DROP DEFAULT;", + "ALTER TABLE users ALTER COLUMN name SET STATISTICS 100;", + "ALTER TABLE users ALTER COLUMN name SET STORAGE PLAIN;", + "ALTER TABLE users ALTER COLUMN name SET COMPRESSION pglz;", + "ALTER TABLE users ALTER COLUMN name DROP EXPRESSION;", + "ALTER TABLE users ALTER COLUMN id ADD GENERATED ALWAYS AS IDENTITY;", + "ALTER TABLE users ALTER COLUMN id SET GENERATED BY DEFAULT;", + "ALTER TABLE users ALTER COLUMN id SET INCREMENT BY 2;", + "ALTER TABLE users ALTER COLUMN id RESTART;", + "ALTER TABLE users ALTER COLUMN id DROP IDENTITY;", + "ALTER TABLE users ALTER COLUMN id DROP IDENTITY IF EXISTS;", + "ALTER TABLE users ENABLE ROW LEVEL SECURITY;", + "ALTER TABLE users DISABLE ROW LEVEL SECURITY;", + "ALTER TABLE users FORCE ROW LEVEL SECURITY;", + "ALTER TABLE users NO FORCE ROW LEVEL SECURITY;", + "ALTER TABLE users ATTACH PARTITION users_p1 FOR VALUES FROM (1) TO (2);", + "ALTER TABLE users DETACH PARTITION users_p1;", + "ALTER TABLE users OWNER TO app;", + "ALTER TABLE users ENABLE TRIGGER users_touch;", + "ALTER TABLE users DISABLE TRIGGER users_touch;", + "ALTER TABLE users ENABLE REPLICA TRIGGER users_touch;", + "ALTER TABLE users ENABLE ALWAYS TRIGGER users_touch;", + "ALTER TABLE users ENABLE RULE users_nodelete;", + "ALTER TABLE users DISABLE RULE users_nodelete;", + "ALTER TABLE users ENABLE REPLICA RULE users_nodelete;", + "ALTER TABLE users ENABLE ALWAYS RULE users_nodelete;", + "ALTER TABLE users VALIDATE CONSTRAINT users_email_key;", + "ALTER TABLE users REPLICA IDENTITY FULL;", + "ALTER TABLE users REPLICA IDENTITY USING INDEX users_email_idx;", + "ALTER TABLE users CLUSTER ON users_email_idx;", + "ALTER TABLE users SET WITHOUT CLUSTER;", + "ALTER TABLE users SET WITHOUT OIDS;", + "ALTER TABLE users SET (fillfactor = 70);", + "ALTER TABLE users SET (fillfactor = 70, autovacuum_enabled = false);", + "ALTER TABLE users RESET (fillfactor);", + "ALTER TABLE users SET TABLESPACE fast;", + "ALTER TABLE users SET LOGGED;", + "ALTER TABLE users SET UNLOGGED;", + "ALTER TABLE users SET ACCESS METHOD heap;", + "ALTER TABLE users INHERIT people;", + "ALTER TABLE users NO INHERIT people;", + "ALTER TABLE users OF person_type;", + "ALTER TABLE users NOT OF;", + "alter table users owner to app;", + "alter table users alter column name set statistics 100;", + "ALTER TABLE ONLY public.users OWNER TO app;", + "ALTER TABLE ONLY public.users ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY;", + "ALTER TABLE users OWNER TO app, VALIDATE CONSTRAINT users_email_key;" + } + ) + void shouldIgnoreTheAlterTableActionsThatCannotChangeAColumn(String statement) { + Schema base = parser.parse(BASE_SCHEMA); + Schema schema = applied(statement); - UnsupportedOperationException exception = assertThrows( - UnsupportedOperationException.class, - () -> parser.parse(schema, "ALTER TABLE users ALTER COLUMN name SET DEFAULT 'new';") + assertEquals(base.tables(), schema.tables()); + assertEquals(base.enums(), schema.enums()); + } + + /** + * A modeled action beside ignored column and table actions of one + * {@code ALTER TABLE} is still applied. + */ + @Test + void shouldApplyAnAddedColumnBesideIgnoredActions() { + Schema schema = applied( + "ALTER TABLE users ADD COLUMN age INTEGER, " + + "ALTER COLUMN name SET DEFAULT 'new', " + + "ALTER COLUMN name SET STATISTICS 100, " + + "OWNER TO app;" ); assertEquals( - "Unsupported ALTER TABLE action at line 1", - exception.getMessage() + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("email", ColumnType.VARCHAR, true), + new Column("name", ColumnType.VARCHAR, true), + new Column("age", ColumnType.INTEGER, true) + ), + table(schema, "users").columns() ); } /** - * An {@code ALTER COLUMN} action that states column storage instead of a new - * type is rejected rather than retyping the column. + * A modeled action written after a table-level action the parser has a form + * of its own for is applied as well. */ @Test - void shouldRejectAlterColumnSetStatistics() { - Schema schema = parser.parse(BASE_SCHEMA); + void shouldApplyAnAddedColumnAfterARowLevelSecurityAction() { + Schema schema = applied("ALTER TABLE users ENABLE ROW LEVEL SECURITY, ADD COLUMN x int;"); - UnsupportedOperationException exception = assertThrows( - UnsupportedOperationException.class, - () -> parser.parse(schema, "ALTER TABLE users ALTER COLUMN name SET STATISTICS 100;") + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("email", ColumnType.VARCHAR, true), + new Column("name", ColumnType.VARCHAR, true), + new Column("x", ColumnType.INTEGER, true) + ), + table(schema, "users").columns() ); + } + /** + * An ignored action still resolves its table, so an {@code ALTER TABLE} of a + * table the schema does not model fails, and {@code IF EXISTS} skips it. + */ + @Test + void shouldReportTheMissingTableOfAnIgnoredAction() { assertEquals( - "Unsupported ALTER TABLE action at line 1", - exception.getMessage() + "Table not found in schema: payments", + failureMessage("ALTER TABLE payments OWNER TO app;") + ); + + assertEquals( + parser.parse(BASE_SCHEMA).tables(), + applied("ALTER TABLE IF EXISTS payments OWNER TO app;").tables() ); } /** - * An {@code ALTER COLUMN} identity action, which the parser reports without - * any declared type, is rejected rather than failing on its missing type. + * An ignored {@code ALTER COLUMN} action does not resolve its column, so it + * is accepted for a column the schema does not model. */ @Test - void shouldRejectAlterColumnAddIdentity() { - Schema schema = parser.parse(BASE_SCHEMA); + void shouldIgnoreAnAlterColumnActionOnAMissingColumn() { + assertEquals( + parser.parse(BASE_SCHEMA).tables(), + applied("ALTER TABLE users ALTER COLUMN nickname SET STATISTICS 100;").tables() + ); + } - UnsupportedOperationException exception = assertThrows( - UnsupportedOperationException.class, - () -> parser.parse( - schema, - "ALTER TABLE users ALTER COLUMN id ADD GENERATED ALWAYS AS IDENTITY;" - ) + /** + * A table-level {@code EXCLUDE} constraint is accepted and not recorded, + * named or unnamed, leaving the columns and the constraints beside it + * modeled. + */ + @ParameterizedTest + @ValueSource( + strings = { + "CONSTRAINT rooms_during_excl EXCLUDE USING gist (during WITH &&)", + "EXCLUDE USING gist (during WITH &&)" + } + ) + void shouldIgnoreExcludeTableConstraints(String constraint) { + Schema schema = parser.parse(""" + CREATE TABLE rooms ( + id BIGINT NOT NULL, + during tsrange, + PRIMARY KEY (id), + %s + ); + """.formatted(constraint)); + + Table rooms = table(schema, "rooms"); + + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("during", null, true, "TSRANGE") + ), + rooms.columns() ); assertEquals( - "Unsupported ALTER TABLE action at line 1", - exception.getMessage() + List.of(new Constraint(ConstraintType.PRIMARY_KEY, List.of("id"))), + rooms.constraints() ); } @@ -1149,8 +1267,18 @@ void shouldApplyAModeledActionBesideAnIgnoredConstraintAction() { delimiter = '|', quoteCharacter = '"', value = { - "ALTER TABLE users ALTER COLUMN name SET DEFAULT 'new';" - + "|Unsupported ALTER TABLE action at line 1", + "ALTER TABLE users SET SCHEMA archive;" + + "|Unsupported ALTER TABLE action: SET SCHEMA archive at line 1", + "ALTER TABLE users ADD COLUMN x int, SET SCHEMA archive;" + + "|Unsupported ALTER TABLE action: SET SCHEMA archive at line 1", + "ALTER TABLE users OWNER TO app, ADD COLUMN x int;" + + "|Unsupported ALTER TABLE action: ADD COLUMN x int at line 1", + "ALTER TABLE users ALTER COLUMN name SET VISIBLE;" + + "|Unsupported ALTER TABLE action: ALTER COLUMN name SET VISIBLE at line 1", + "ALTER TABLE users ALTER COLUMN name SET FOO 1;" + + "|Unsupported ALTER TABLE action: ALTER COLUMN name SET FOO 1 at line 1", + "ALTER TABLE users ALTER COLUMN name text;" + + "|Unsupported ALTER TABLE action: ALTER COLUMN name text at line 1", "ALTER TYPE status RENAME TO state;|Unsupported ALTER TYPE action at line 1", "ALTER TYPE status RENAME VALUE 'draft' TO 'new';" + "|Unsupported ALTER TYPE action at line 1", @@ -1171,6 +1299,29 @@ void shouldRejectUnsupportedStatementsInSqlTerms(String statement, String messag assertEquals(message, exception.getMessage()); } + /** + * A rejected {@code ALTER TABLE} action is reported at the line its own + * statement begins on, after the ignored statements before it. + */ + @Test + void shouldReportTheLineOfARejectedAlterTableAction() { + Schema schema = parser.parse(BASE_SCHEMA); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> parser.parse(schema, """ + ALTER TABLE users OWNER TO app; + + ALTER TABLE users SET SCHEMA archive; + """) + ); + + assertEquals( + "Unsupported ALTER TABLE action: SET SCHEMA archive at line 3", + exception.getMessage() + ); + } + /** * A function body that is not dollar-quoted ends at its own separator, so it * is ignored like any other function and the statements after it, including a