From c5da46535ad28876d17b3c4d3bf585e7513b6ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20=C3=87engel?= Date: Sun, 27 Sep 2026 23:18:11 +0300 Subject: [PATCH 1/2] feat(schema): apply ordered CREATE, ALTER, and DROP TABLE statements --- .../dev/sqlcj/compiler/SqlcjCompiler.java | 16 +- .../schema/parser/DefaultSchemaParser.java | 419 ++++++++++++++- .../dev/sqlcj/schema/parser/SchemaParser.java | 14 +- .../SqlcjCompilerIntegrationTest.java | 186 +++++++ .../parser/DefaultSchemaParserTest.java | 492 +++++++++++++++++- 5 files changed, 1106 insertions(+), 21 deletions(-) diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/compiler/SqlcjCompiler.java b/sqlcj-cli/src/main/java/dev/sqlcj/compiler/SqlcjCompiler.java index 8bf28fb..1ed8152 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/compiler/SqlcjCompiler.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/compiler/SqlcjCompiler.java @@ -11,7 +11,6 @@ import dev.sqlcj.io.GeneratedOutputManifest; import dev.sqlcj.parser.Query; import dev.sqlcj.schema.Schema; -import dev.sqlcj.schema.Table; import dev.sqlcj.schema.parser.DefaultSchemaParser; import dev.sqlcj.schema.parser.SchemaParser; import dev.sqlcj.sql.ParsedSql; @@ -91,22 +90,23 @@ private List generate(List sources, CodeGenerator codeGen } /** - * Parses the schema files of one entry in order into that entry's schema, - * so a table keeps the position of the file that declares it. + * Applies the schema files of one entry in order to that entry's schema, so + * a table keeps the position of the file that declares it and a later file + * alters what the earlier files left. */ private Schema parseSchema(Source source) { - List tables = new ArrayList<>(); + Schema schema = new Schema(List.of()); for (SchemaFile schemaFile : source.schemaFiles()) { - tables.addAll(parseSchemaFile(schemaFile).tables()); + schema = parseSchemaFile(schema, schemaFile); } - return new Schema(tables); + return schema; } - private Schema parseSchemaFile(SchemaFile schemaFile) { + private Schema parseSchemaFile(Schema schema, SchemaFile schemaFile) { try { - return schemaParser.parse(schemaFile.sql()); + return schemaParser.parse(schema, schemaFile.sql()); } catch (RuntimeException e) { throw new CompilationException( "Invalid schema source %s: %s".formatted(schemaFile.path(), reason(e)), 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 49e971e..3b399f5 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 @@ -12,11 +12,15 @@ import net.sf.jsqlparser.schema.MultiPartName; import net.sf.jsqlparser.statement.Statement; import net.sf.jsqlparser.statement.Statements; +import net.sf.jsqlparser.statement.alter.Alter; +import net.sf.jsqlparser.statement.alter.AlterExpression; +import net.sf.jsqlparser.statement.alter.AlterOperation; 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.ForeignKeyIndex; import net.sf.jsqlparser.statement.create.table.Index; +import net.sf.jsqlparser.statement.drop.Drop; import java.util.ArrayList; import java.util.List; @@ -29,21 +33,14 @@ public class DefaultSchemaParser implements SchemaParser { private static final Pattern TYPE_ARGUMENTS = Pattern.compile("\\([^)]*\\)"); @Override - public Schema parse(String sql) { + public Schema parse(Schema schema, String sql) { try { Statements statements = CCJSqlParserUtil.parseStatements(sql); - List
tables = new ArrayList<>(); + List
tables = new ArrayList<>(schema.tables()); for (Statement statement : statements) { - if (!(statement instanceof CreateTable createTable)) { - throw new UnsupportedOperationException( - "Unsupported schema statement: " - + statement.getClass().getSimpleName() - ); - } - - tables.add(parseTable(createTable)); + apply(statement, tables); } return new Schema(tables); @@ -52,6 +49,400 @@ public Schema parse(String sql) { } } + /** Applies one statement to the tables the statements before it left. */ + private void apply(Statement statement, List
tables) { + if (statement instanceof CreateTable createTable) { + applyCreateTable(createTable, tables); + } else if (statement instanceof Alter alter) { + applyAlterTable(alter, tables); + } else if (statement instanceof Drop drop && drop.getObjectType() == Drop.ObjectType.TABLE) { + applyDropTable(drop, tables); + } else { + throw unsupportedStatement(statement); + } + } + + /** + * Appends one table. A repeated table name fails, as PostgreSQL rejects it, + * unless the statement declares {@code IF NOT EXISTS}. + */ + private void applyCreateTable(CreateTable createTable, List
tables) { + String tableName = createTable.getTable().getUnquotedName(); + + if (indexOfTable(tables, tableName) >= 0) { + if (createTable.isIfNotExists()) { + return; + } + + throw tableAlreadyExists(tableName); + } + + tables.add(parseTable(createTable)); + } + + /** Removes every named table, keeping the order of the remaining ones. */ + private void applyDropTable(Drop drop, List
tables) { + for (net.sf.jsqlparser.schema.Table dropped : drop.getNames()) { + String tableName = dropped.getUnquotedName(); + + int index = indexOfTable(tables, tableName); + + if (index < 0) { + if (drop.isIfExists()) { + continue; + } + + throw tableNotFound(tableName); + } + + tables.remove(index); + } + } + + /** + * Applies the actions of one {@code ALTER TABLE} in order, in the position + * of the altered table. {@code ALTER TABLE IF EXISTS} covers only the table, + * so a missing column of an existing table still fails. + */ + private void applyAlterTable(Alter alter, List
tables) { + String tableName = alter.getTable().getUnquotedName(); + + int index = indexOfTable(tables, tableName); + + if (index < 0) { + if (alter.isUseTableIfExists()) { + return; + } + + throw tableNotFound(tableName); + } + + for (AlterExpression expression : alter.getAlterExpressions()) { + tables.set(index, applyAlterExpression(alter, expression, tables, index)); + } + } + + /** + * Applies one {@code ALTER TABLE} action and returns the altered table. An + * action that cannot be recognized as one of the supported forms, such as + * {@code SET DEFAULT} or {@code ADD CONSTRAINT}, is rejected like any other + * unsupported statement. + */ + private Table applyAlterExpression( + Alter alter, + AlterExpression expression, + List
tables, + int index + ) { + Table table = tables.get(index); + AlterOperation operation = expression.getOperation(); + + if (operation == AlterOperation.ADD && isNotEmpty(expression.getColDataTypeList())) { + return addColumns(expression, table); + } + + if (operation == AlterOperation.DROP && expression.getColumnName() != null) { + return dropColumn(expression, table); + } + + if (operation == AlterOperation.RENAME && expression.getColumnOldName() != null) { + return renameColumn(expression, table); + } + + if (operation == AlterOperation.RENAME_TABLE) { + return renameTable(expression, tables, index); + } + + if (operation == AlterOperation.ALTER) { + return alterColumns(alter, expression, table); + } + + throw unsupportedStatement(alter); + } + + /** + * Applies one {@code ALTER COLUMN} action: a new type, which keeps the + * column's position and nullability, or a nullability change. + */ + private Table alterColumns(Alter alter, AlterExpression expression, Table table) { + if (isNotEmpty(expression.getColDataTypeList())) { + if (!statesNewTypes(expression.getColDataTypeList())) { + throw unsupportedStatement(alter); + } + + return changeColumnTypes(expression, table); + } + + if (isNotEmpty(expression.getColumnSetNotNullList())) { + return changeNullability( + expression.getColumnSetNotNullList().stream() + .map(AlterExpression.ColumnSetNotNull::getColumnName) + .toList(), + false, + table + ); + } + + if (isNotEmpty(expression.getColumnDropNotNullList())) { + return changeNullability( + expression.getColumnDropNotNullList().stream() + .map(AlterExpression.ColumnDropNotNull::getColumnName) + .toList(), + true, + table + ); + } + + throw unsupportedStatement(alter); + } + + /** + * 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. + */ + private boolean statesNewTypes(List definitions) { + return definitions.stream() + .allMatch( + definition -> definition.isWithType() && definition.getColDataType() != null + ); + } + + /** + * Appends each added column, typed exactly as a {@code CREATE TABLE} column + * of the same declaration, and records its column-level constraints. + */ + private Table addColumns(AlterExpression expression, Table table) { + List columns = new ArrayList<>(table.columns()); + List constraints = new ArrayList<>(table.constraints()); + + for (AlterExpression.ColumnDataType definition : expression.getColDataTypeList()) { + Column column = parseColumn(definition); + + if (indexOfColumn(columns, column.name()) >= 0) { + if (expression.isUseIfNotExists()) { + continue; + } + + throw columnAlreadyExists(table.name(), column.name()); + } + + columns.add(column); + constraints.addAll(parseColumnConstraints(definition)); + } + + return new Table(table.name(), columns, constraints); + } + + /** Removes one column and every constraint that lists it. */ + private Table dropColumn(AlterExpression expression, Table table) { + String columnName = MultiPartName.unquote(expression.getColumnName()); + + int index = indexOfColumn(table.columns(), columnName); + + if (index < 0) { + if (expression.isUsingIfExists()) { + return table; + } + + throw columnNotFound(table.name(), columnName); + } + + List columns = new ArrayList<>(table.columns()); + + columns.remove(index); + + List constraints = table.constraints().stream() + .filter(constraint -> !listsColumn(constraint, columnName)) + .toList(); + + return new Table(table.name(), columns, constraints); + } + + /** + * Renames one column in its position and in the constraints that list it. + */ + private Table renameColumn(AlterExpression expression, Table table) { + String oldName = MultiPartName.unquote(expression.getColumnOldName()); + String newName = MultiPartName.unquote(expression.getColumnName()); + + int index = indexOfColumn(table.columns(), oldName); + + if (index < 0) { + throw columnNotFound(table.name(), oldName); + } + + int existing = indexOfColumn(table.columns(), newName); + + if (existing >= 0 && existing != index) { + throw columnAlreadyExists(table.name(), newName); + } + + List columns = new ArrayList<>(table.columns()); + Column column = columns.get(index); + + columns.set( + index, + new Column(newName, column.type(), column.nullable(), column.unsupportedType()) + ); + + List constraints = table.constraints().stream() + .map(constraint -> renameConstraintColumn(constraint, oldName, newName)) + .toList(); + + return new Table(table.name(), columns, constraints); + } + + /** Renames one table in its position, keeping its columns and constraints. */ + private Table renameTable(AlterExpression expression, List
tables, int index) { + String newName = MultiPartName.unquote(expression.getNewTableName()); + + int existing = indexOfTable(tables, newName); + + if (existing >= 0 && existing != index) { + throw tableAlreadyExists(newName); + } + + Table table = tables.get(index); + + return new Table(newName, table.columns(), table.constraints()); + } + + /** + * Retypes each named column in its position, mapping or recording the new + * type as a {@code CREATE TABLE} column of the same type would be, and + * keeping the column's nullability, which a type change does not state. + */ + private Table changeColumnTypes(AlterExpression expression, Table table) { + List columns = new ArrayList<>(table.columns()); + + for (AlterExpression.ColumnDataType definition : expression.getColDataTypeList()) { + String columnName = MultiPartName.unquote(definition.getColumnName()); + + int index = indexOfColumn(columns, columnName); + + if (index < 0) { + throw columnNotFound(table.name(), columnName); + } + + Column column = columns.get(index); + Column retyped = parseColumn(definition); + + columns.set( + index, + new Column( + column.name(), + retyped.type(), + column.nullable(), + retyped.unsupportedType() + ) + ); + } + + return new Table(table.name(), columns, table.constraints()); + } + + /** Sets the nullability of each named column in its position. */ + private Table changeNullability( + List declaredNames, + boolean nullable, + Table table + ) { + List columns = new ArrayList<>(table.columns()); + + for (String declaredName : declaredNames) { + String columnName = MultiPartName.unquote(declaredName); + + int index = indexOfColumn(columns, columnName); + + if (index < 0) { + throw columnNotFound(table.name(), columnName); + } + + Column column = columns.get(index); + + columns.set( + index, + new Column(column.name(), column.type(), nullable, column.unsupportedType()) + ); + } + + return new Table(table.name(), columns, table.constraints()); + } + + private boolean listsColumn(Constraint constraint, String columnName) { + return constraint.columns().stream() + .anyMatch(column -> column.equalsIgnoreCase(columnName)); + } + + private Constraint renameConstraintColumn( + Constraint constraint, + String oldName, + String newName + ) { + return new Constraint( + constraint.type(), + constraint.columns().stream() + .map(column -> column.equalsIgnoreCase(oldName) ? newName : column) + .toList() + ); + } + + /** + * Table and column names are matched case-insensitively, as query analysis + * looks them up. + */ + private int indexOfTable(List
tables, String tableName) { + for (int i = 0; i < tables.size(); i++) { + if (tables.get(i).name().equalsIgnoreCase(tableName)) { + return i; + } + } + + return -1; + } + + private int indexOfColumn(List columns, String columnName) { + for (int i = 0; i < columns.size(); i++) { + if (columns.get(i).name().equalsIgnoreCase(columnName)) { + return i; + } + } + + return -1; + } + + private boolean isNotEmpty(List list) { + return list != null && !list.isEmpty(); + } + + private IllegalArgumentException tableNotFound(String tableName) { + return new IllegalArgumentException("Table not found in schema: " + tableName); + } + + private IllegalArgumentException tableAlreadyExists(String tableName) { + return new IllegalArgumentException("Table already exists in schema: " + tableName); + } + + private IllegalArgumentException columnNotFound(String tableName, String columnName) { + return new IllegalArgumentException( + "Column not found in table %s: %s".formatted(tableName, columnName) + ); + } + + private IllegalArgumentException columnAlreadyExists(String tableName, String columnName) { + return new IllegalArgumentException( + "Column already exists in table %s: %s".formatted(tableName, columnName) + ); + } + + private UnsupportedOperationException unsupportedStatement(Statement statement) { + return new UnsupportedOperationException( + "Unsupported schema statement: " + statement.getClass().getSimpleName() + ); + } + private Table parseTable(CreateTable createTable) { String tableName = createTable.getTable().getUnquotedName(); @@ -59,7 +450,13 @@ private Table parseTable(CreateTable createTable) { List constraints = new ArrayList<>(); for (ColumnDefinition definition : createTable.getColumnDefinitions()) { - columns.add(parseColumn(definition)); + Column column = parseColumn(definition); + + if (indexOfColumn(columns, column.name()) >= 0) { + throw columnAlreadyExists(tableName, column.name()); + } + + columns.add(column); constraints.addAll(parseColumnConstraints(definition)); } diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaParser.java b/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaParser.java index 40749ac..efb99b8 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaParser.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/schema/parser/SchemaParser.java @@ -2,7 +2,19 @@ import dev.sqlcj.schema.Schema; +import java.util.List; + public interface SchemaParser { - Schema parse(String sql); + /** + * Applies the statements of one schema source, in order, to {@code schema} + * and returns the schema they leave, so a statement of a later source sees + * the tables of the earlier ones. + */ + Schema parse(Schema schema, String sql); + + /** Applies one schema source to an empty schema. */ + default Schema parse(String sql) { + return parse(new Schema(List.of()), sql); + } } diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java index 502c8c6..4ad1000 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java @@ -2259,6 +2259,192 @@ total DECIMAL(10, 2) ); } + /** + * A later migration alters the tables the earlier ones created, so the + * migration directory and the snapshot of the altered tables describe one + * schema and generate the same repository source. + */ + @Test + void shouldGenerateTheSameRepositoryFromAlteringMigrationsAndASnapshot() + throws IOException { + Path migrations = Files.createDirectories(tempDir.resolve("migrations")); + + Files.writeString( + migrations.resolve("V1__users.sql"), + """ + CREATE TABLE users + ( + id BIGINT NOT NULL, + email VARCHAR(255), + name VARCHAR(255) NOT NULL, + note TEXT + ); + """ + ); + + Files.writeString( + migrations.resolve("V2__orders.sql"), + """ + CREATE TABLE orders + ( + id BIGINT NOT NULL, + user_id BIGINT NOT NULL, + total DECIMAL(10, 2) + ); + """ + ); + + Files.writeString( + migrations.resolve("V3__alter_users.sql"), + """ + ALTER TABLE users RENAME COLUMN email TO email_address; + ALTER TABLE users ADD COLUMN active BOOLEAN NOT NULL; + ALTER TABLE users DROP COLUMN note; + ALTER TABLE users ALTER COLUMN id TYPE INT4; + ALTER TABLE users ALTER COLUMN name DROP NOT NULL; + ALTER TABLE orders ALTER COLUMN total SET NOT NULL; + """ + ); + + Path snapshotFile = tempDir.resolve("schema.sql"); + + Files.writeString( + snapshotFile, + """ + CREATE TABLE users + ( + id INT4 NOT NULL, + email_address VARCHAR(255), + name VARCHAR(255), + active BOOLEAN NOT NULL + ); + + CREATE TABLE orders + ( + id BIGINT NOT NULL, + user_id BIGINT NOT NULL, + total DECIMAL(10, 2) NOT NULL + ); + """ + ); + + Path queriesFile = tempDir.resolve("queries.sql"); + + Files.writeString( + queriesFile, + """ + -- name: ListUsers :many + SELECT * + FROM users; + + -- name: FindOrder :optional + SELECT * + FROM orders + WHERE id = $1; + + -- name: ListActiveUsers :many + SELECT id, email_address + FROM users + WHERE active = $1; + """ + ); + + Path fromDirectory = generateUsersRepository( + List.of(migrations.toString()), + queriesFile, + tempDir.resolve("generated-directory") + ); + + Path fromSnapshot = generateUsersRepository( + List.of(snapshotFile.toString()), + queriesFile, + tempDir.resolve("generated-snapshot") + ); + + assertArrayEquals( + Files.readAllBytes(fromSnapshot), + Files.readAllBytes(fromDirectory) + ); + + Path classesDirectory = tempDir.resolve("classes"); + + Files.createDirectories(classesDirectory); + + JavaCompiler compilerApi = ToolProvider.getSystemJavaCompiler(); + + assertNotNull(compilerApi); + + assertEquals( + 0, + compilerApi.run( + null, + null, + null, + "-classpath", + System.getProperty("java.class.path"), + "-d", + classesDirectory.toString(), + fromDirectory.toString() + ) + ); + } + + /** + * A migration that alters a table no earlier migration created names the + * file that refers to it. + */ + @Test + void shouldReportTheMigrationFileThatAltersAMissingTable() throws IOException { + Path migrations = Files.createDirectories(tempDir.resolve("migrations")); + + Files.writeString( + migrations.resolve("V1__users.sql"), + """ + CREATE TABLE users + ( + id BIGINT NOT NULL + ); + """ + ); + + Path missingReference = migrations.resolve("V2__payments.sql"); + + Files.writeString( + missingReference, + "ALTER TABLE payments ADD COLUMN total DECIMAL(10, 2);\n" + ); + + Path queriesFile = tempDir.resolve("queries.sql"); + + Files.writeString( + queriesFile, + """ + -- name: ListUsers :many + SELECT id + FROM users; + """ + ); + + Path generatedDirectory = tempDir.resolve("generated"); + + CompilationException exception = assertThrows( + CompilationException.class, + () -> generateUsersRepository( + List.of(migrations.toString()), + queriesFile, + generatedDirectory + ) + ); + + assertEquals( + "Invalid schema source %s: ".formatted(missingReference) + + "Table not found in schema: payments", + exception.getMessage() + ); + + assertFalse(Files.exists(generatedDirectory)); + } + /** A schema failure names the migration file that contains it. */ @Test void shouldReportTheMigrationFileThatFailsToParse() throws IOException { 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 6fa578d..fcfc703 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 @@ -20,6 +20,20 @@ class DefaultSchemaParserTest { + /** The schema the ordered-DDL tests apply their statements to. */ + private static final String BASE_SCHEMA = """ + CREATE TABLE users ( + id BIGINT NOT NULL, + email VARCHAR(255) UNIQUE, + name VARCHAR(255) + ); + + CREATE TABLE orders ( + id BIGINT NOT NULL, + user_id BIGINT NOT NULL + ); + """; + private final SchemaParser parser = new DefaultSchemaParser(); @Test @@ -529,7 +543,7 @@ CREATE TABLE users ( id BIGINT NOT NULL ); - ALTER TABLE users ADD COLUMN name VARCHAR(255); + CREATE VIEW active_users AS SELECT id FROM users; """; assertThrows( @@ -538,6 +552,453 @@ CREATE TABLE users ( ); } + /** + * An {@code ALTER TABLE} action outside the supported forms, such as + * {@code SET DEFAULT}, is rejected like any other unsupported statement. + */ + @Test + void shouldRejectUnsupportedAlterTableAction() { + Schema schema = parser.parse(BASE_SCHEMA); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> parser.parse(schema, "ALTER TABLE users ALTER COLUMN name SET DEFAULT 'new';") + ); + + assertEquals( + "Unsupported schema statement: Alter", + exception.getMessage() + ); + } + + /** + * An {@code ALTER COLUMN} action that states column storage instead of a new + * type is rejected rather than retyping the column. + */ + @Test + void shouldRejectAlterColumnSetStatistics() { + Schema schema = parser.parse(BASE_SCHEMA); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> parser.parse(schema, "ALTER TABLE users ALTER COLUMN name SET STATISTICS 100;") + ); + + assertEquals( + "Unsupported schema statement: Alter", + exception.getMessage() + ); + } + + /** + * An {@code ALTER COLUMN} identity action, which the parser reports without + * any declared type, is rejected rather than failing on its missing type. + */ + @Test + void shouldRejectAlterColumnAddIdentity() { + Schema schema = parser.parse(BASE_SCHEMA); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> parser.parse( + schema, + "ALTER TABLE users ALTER COLUMN id ADD GENERATED ALWAYS AS IDENTITY;" + ) + ); + + assertEquals( + "Unsupported schema statement: Alter", + exception.getMessage() + ); + } + + /** + * Each source is applied to the schema the previous ones left, and leaves + * their schema untouched. + */ + @Test + void shouldComposeTheSchemaOfSeveralParsedSources() { + Schema first = parser.parse(""" + CREATE TABLE users ( + id BIGINT NOT NULL + ); + """); + + Schema second = parser.parse( + first, + "ALTER TABLE users ADD COLUMN name VARCHAR(255);" + ); + + Schema third = parser.parse(second, "ALTER TABLE users RENAME TO people;"); + + assertEquals(List.of("people"), tableNames(third)); + + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("name", ColumnType.VARCHAR, true) + ), + third.tables().getFirst().columns() + ); + + assertEquals(List.of("users"), tableNames(first)); + + assertEquals( + List.of(new Column("id", ColumnType.BIGINT, false)), + first.tables().getFirst().columns() + ); + } + + @Test + void shouldAppendAddedColumnsTypedLikeCreateTableColumns() { + Schema schema = applied(""" + ALTER TABLE users ADD COLUMN created_at TIMESTAMPTZ NOT NULL; + ALTER TABLE users ADD COLUMN revision SERIAL; + ALTER TABLE users ADD COLUMN metadata JSONB; + ALTER TABLE users ADD COLUMN tags varchar(20)[]; + """); + + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("email", ColumnType.VARCHAR, true), + new Column("name", ColumnType.VARCHAR, true), + new Column("created_at", ColumnType.TIMESTAMP_WITH_TIME_ZONE, false), + new Column("revision", ColumnType.INTEGER, false), + new Column("metadata", null, true, "JSONB"), + new Column("tags", null, true, "VARCHAR[]") + ), + table(schema, "users").columns() + ); + } + + @Test + void shouldRecordTheColumnConstraintsOfAnAddedColumn() { + Schema schema = applied(""" + ALTER TABLE orders ADD COLUMN code VARCHAR(32) UNIQUE; + ALTER TABLE orders ADD COLUMN reference BIGINT PRIMARY KEY; + """); + + assertEquals( + List.of( + new Constraint(ConstraintType.UNIQUE, List.of("code")), + new Constraint(ConstraintType.PRIMARY_KEY, List.of("reference")) + ), + table(schema, "orders").constraints() + ); + } + + @Test + void shouldDropColumnAndTheConstraintsThatListIt() { + Schema schema = applied("ALTER TABLE users DROP COLUMN email;"); + + Table users = table(schema, "users"); + + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("name", ColumnType.VARCHAR, true) + ), + users.columns() + ); + + assertTrue(users.constraints().isEmpty()); + } + + @Test + void shouldRenameColumnInItsPositionAndInItsConstraints() { + Schema schema = applied( + "ALTER TABLE users RENAME COLUMN email TO \"Email Address\";" + ); + + Table users = table(schema, "users"); + + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("Email Address", ColumnType.VARCHAR, true), + new Column("name", ColumnType.VARCHAR, true) + ), + users.columns() + ); + + assertEquals( + List.of( + new Constraint(ConstraintType.UNIQUE, List.of("Email Address")) + ), + users.constraints() + ); + } + + @Test + void shouldRenameTableInItsPosition() { + Schema schema = applied("ALTER TABLE users RENAME TO \"people\";"); + + assertEquals(List.of("people", "orders"), tableNames(schema)); + + Table people = table(schema, "people"); + + assertEquals(3, people.columns().size()); + + assertEquals( + List.of(new Constraint(ConstraintType.UNIQUE, List.of("email"))), + people.constraints() + ); + } + + /** + * A type change states no nullability, so the column keeps the nullability + * it had, whether its new type is mapped or recorded. + */ + @Test + void shouldChangeColumnTypeInItsPositionKeepingItsNullability() { + Schema schema = applied(""" + ALTER TABLE users ALTER COLUMN id TYPE INT4; + ALTER TABLE users ALTER COLUMN email TYPE JSONB; + """); + + assertEquals( + List.of( + new Column("id", ColumnType.INTEGER, false), + new Column("email", null, true, "JSONB"), + new Column("name", ColumnType.VARCHAR, true) + ), + table(schema, "users").columns() + ); + } + + @Test + void shouldChangeARecordedColumnTypeBackToAMappedType() { + Schema schema = applied(""" + ALTER TABLE users ALTER COLUMN name TYPE JSONB; + ALTER TABLE users ALTER COLUMN name SET NOT NULL; + ALTER TABLE users ALTER COLUMN name TYPE TEXT; + """); + + assertEquals( + new Column("name", ColumnType.TEXT, false), + table(schema, "users").columns().get(2) + ); + } + + @Test + void shouldSetAndDropColumnNullability() { + Schema schema = applied(""" + ALTER TABLE users ALTER COLUMN name SET NOT NULL; + ALTER TABLE users ALTER COLUMN id DROP NOT NULL; + """); + + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, true), + new Column("email", ColumnType.VARCHAR, true), + new Column("name", ColumnType.VARCHAR, false) + ), + table(schema, "users").columns() + ); + } + + @Test + void shouldApplyTheActionsOfOneAlterTableInOrder() { + Schema schema = applied(""" + ALTER TABLE users + DROP COLUMN email, + ADD COLUMN status VARCHAR(32) NOT NULL, + ALTER COLUMN name SET NOT NULL, + ALTER COLUMN id TYPE INT4; + """); + + Table users = table(schema, "users"); + + assertEquals( + List.of( + new Column("id", ColumnType.INTEGER, false), + new Column("name", ColumnType.VARCHAR, false), + new Column("status", ColumnType.VARCHAR, false) + ), + users.columns() + ); + + assertTrue(users.constraints().isEmpty()); + } + + @Test + void shouldDropEveryTableOfOneDropStatement() { + Schema schema = applied("DROP TABLE orders, users;"); + + assertTrue(schema.tables().isEmpty()); + } + + @Test + void shouldIgnoreCreateTableIfNotExistsForAnExistingTable() { + Schema schema = applied(""" + CREATE TABLE IF NOT EXISTS users ( + other BIGINT NOT NULL + ); + """); + + assertEquals(List.of("users", "orders"), tableNames(schema)); + + assertEquals(3, table(schema, "users").columns().size()); + } + + @Test + void shouldIgnoreAddColumnIfNotExistsForAnExistingColumn() { + Schema schema = applied("ALTER TABLE users ADD COLUMN IF NOT EXISTS name TEXT;"); + + Table users = table(schema, "users"); + + assertEquals(3, users.columns().size()); + + assertEquals( + new Column("name", ColumnType.VARCHAR, true), + users.columns().get(2) + ); + } + + @Test + void shouldIgnoreDropTableIfExistsForAMissingTable() { + Schema schema = applied("DROP TABLE IF EXISTS payments, orders;"); + + assertEquals(List.of("users"), tableNames(schema)); + } + + @Test + void shouldIgnoreAlterTableIfExistsForAMissingTable() { + Schema schema = applied( + "ALTER TABLE IF EXISTS payments ADD COLUMN total DECIMAL(10, 2);" + ); + + assertEquals(parser.parse(BASE_SCHEMA).tables(), schema.tables()); + } + + @Test + void shouldIgnoreDropColumnIfExistsForAMissingColumn() { + Schema schema = applied("ALTER TABLE users DROP COLUMN IF EXISTS nickname;"); + + Table users = table(schema, "users"); + + assertEquals(3, users.columns().size()); + + assertEquals( + List.of(new Constraint(ConstraintType.UNIQUE, List.of("email"))), + users.constraints() + ); + } + + /** + * {@code ALTER TABLE IF EXISTS} covers only the table, so a missing column + * of an existing table still fails. + */ + @Test + void shouldReportTheMissingColumnOfAnAlterTableIfExists() { + assertEquals( + "Column not found in table users: nickname", + failureMessage("ALTER TABLE IF EXISTS users DROP COLUMN nickname;") + ); + } + + @Test + void shouldMatchTableAndColumnNamesCaseInsensitively() { + Schema schema = applied(""" + ALTER TABLE USERS ALTER COLUMN NAME SET NOT NULL; + ALTER TABLE Users RENAME COLUMN Email TO Mail; + DROP TABLE Orders; + """); + + assertEquals(List.of("users"), tableNames(schema)); + + assertEquals( + List.of( + new Column("id", ColumnType.BIGINT, false), + new Column("Mail", ColumnType.VARCHAR, true), + new Column("name", ColumnType.VARCHAR, false) + ), + table(schema, "users").columns() + ); + } + + @Test + void shouldReportTheMissingTableOfAStatement() { + assertEquals( + "Table not found in schema: payments", + failureMessage("ALTER TABLE payments ADD COLUMN total DECIMAL(10, 2);") + ); + + assertEquals( + "Table not found in schema: payments", + failureMessage("DROP TABLE payments;") + ); + } + + @Test + void shouldReportTheMissingColumnOfAnAlterTableAction() { + assertEquals( + "Column not found in table users: nickname", + failureMessage("ALTER TABLE users DROP COLUMN nickname;") + ); + + assertEquals( + "Column not found in table users: nickname", + failureMessage("ALTER TABLE users RENAME COLUMN nickname TO handle;") + ); + + assertEquals( + "Column not found in table users: nickname", + failureMessage("ALTER TABLE users ALTER COLUMN nickname TYPE TEXT;") + ); + + assertEquals( + "Column not found in table users: nickname", + failureMessage("ALTER TABLE users ALTER COLUMN nickname SET NOT NULL;") + ); + + assertEquals( + "Column not found in table users: nickname", + failureMessage("ALTER TABLE users ALTER COLUMN nickname DROP NOT NULL;") + ); + } + + @Test + void shouldReportAStatementThatRepeatsATableName() { + assertEquals( + "Table already exists in schema: users", + failureMessage(""" + CREATE TABLE users ( + id BIGINT NOT NULL + ); + """) + ); + + assertEquals( + "Table already exists in schema: orders", + failureMessage("ALTER TABLE users RENAME TO orders;") + ); + } + + @Test + void shouldReportAStatementThatRepeatsAColumnName() { + assertEquals( + "Column already exists in table users: name", + failureMessage("ALTER TABLE users ADD COLUMN name TEXT;") + ); + + assertEquals( + "Column already exists in table users: name", + failureMessage("ALTER TABLE users RENAME COLUMN email TO name;") + ); + + assertEquals( + "Column already exists in table people: name", + failureMessage(""" + CREATE TABLE people ( + name TEXT, + name VARCHAR(255) + ); + """) + ); + } + @Test void shouldReportSyntaxFailureWithItsReasonAndLocation() { String sql = """ @@ -607,6 +1068,35 @@ CREATE TABLE users ( assertFalse(exception.getMessage().contains("net.sf.jsqlparser")); } + /** The schema {@code statements} leave when applied to the base schema. */ + private Schema applied(String statements) { + return parser.parse(parser.parse(BASE_SCHEMA), statements); + } + + /** The message of the failure {@code statements} cause on the base schema. */ + private String failureMessage(String statements) { + Schema schema = parser.parse(BASE_SCHEMA); + + return assertThrows( + IllegalArgumentException.class, + () -> parser.parse(schema, statements) + ) + .getMessage(); + } + + private List tableNames(Schema schema) { + return schema.tables().stream() + .map(Table::name) + .toList(); + } + + private Table table(Schema schema, String tableName) { + return schema.tables().stream() + .filter(table -> table.name().equals(tableName)) + .findFirst() + .orElseThrow(); + } + @Test void shouldThrowSchemaParseExceptionForInvalidSql() { String sql = """ From 37b2bf5a596003ff62faf51920e58f8bb95bfc42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20=C3=87engel?= Date: Sun, 27 Sep 2026 23:18:18 +0300 Subject: [PATCH 2/2] docs: document ordered table DDL in schema files --- docs/configuration.md | 9 ++-- docs/postgresql.md | 99 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 97 insertions(+), 11 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 918a595..d9c2e51 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -133,11 +133,14 @@ The order never depends on how the filesystem lists the directory. sqlcj only reads the files, so a Flyway placeholder, a configured file-name prefix, and a repeatable migration have no meaning of their own. -Each schema file is parsed on its own, and the tables of the entry follow file -order, so a schema failure names the file that contains it: +The statements of the files are applied in that order to one schema model, so a +later migration alters the tables an earlier file created and a schema failure +names the file that contains it. A statement that refers to a table or a column +no earlier statement created is such a failure; see +[Ordered Table DDL](postgresql.md#ordered-table-ddl): ```text -sqlcj: Invalid schema source /home/dev/project/sql/orders/migrations/V2__add_orders.sql: Unsupported schema statement: Alter +sqlcj: Invalid schema source /home/dev/project/sql/orders/migrations/V2__add_orders.sql: Table not found in schema: payments ``` A directory that contributes no file, and two versioned files of one directory diff --git a/docs/postgresql.md b/docs/postgresql.md index 649cc02..66d4835 100644 --- a/docs/postgresql.md +++ b/docs/postgresql.md @@ -92,10 +92,14 @@ file, a list of files, or a directory of `.sql` migration files; see [`sql[].schema`](configuration.md#sqlschema) for the accepted forms and the order the files are read in. -- Only `CREATE TABLE` statements are accepted. Any other statement in a schema - file is rejected. -- A file may contain several `CREATE TABLE` statements, and table order is - preserved across the schema files of one entry. +- Only `CREATE TABLE`, `DROP TABLE`, and the `ALTER TABLE` forms listed in + [Ordered Table DDL](#ordered-table-ddl) are accepted. Any other statement in a + schema file is rejected. +- The statements of the schema files of one entry are applied in order to one + schema model, so each of them sees the tables and columns the statements and + files before it left. +- A file may contain several statements, and a table keeps the position of the + statement that created it. - SQL identifier delimiters are removed for the parsed model, so the table `"user data"` is modeled as `user data` and the column `"user id"` is modeled as `user id`. @@ -148,7 +152,7 @@ selected-column list. | 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. | -| Any statement other than `CREATE TABLE`, such as `ALTER TABLE` | Rejected. | +| Any statement outside [Ordered Table DDL](#ordered-table-ddl), such as `CREATE VIEW` | Rejected. | | Any other table-constraint kind | Rejected. | | Unparsable SQL | Rejected. | @@ -158,6 +162,46 @@ pipeline reads it, so it changes no generated type, method, parameter, or result component. "Ignored" means the construct is accepted as valid schema input and is not carried into the model at all. +## Ordered Table DDL + +A schema of `CREATE TABLE` statements alone is modeled exactly as it was before +ordered DDL existed. Beyond it, these statements update the schema the +statements and files before them left: + +| Statement | Handling | +| --- | --- | +| `CREATE TABLE` | Adds the table after the tables already modeled. | +| `CREATE TABLE IF NOT EXISTS` | Does nothing when the table already exists. | +| `DROP TABLE`, with one or several names | Removes each named table. | +| `DROP TABLE IF EXISTS` | Does nothing for a name that is not modeled. | +| `ALTER TABLE ... ADD COLUMN` | Appends the column, typed exactly as a `CREATE TABLE` column of the same declaration, and records its column-level `PRIMARY KEY` or `UNIQUE`. | +| `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` | Does nothing when the column already exists. | +| `ALTER TABLE ... DROP COLUMN` | Removes the column and every recorded constraint that lists it. | +| `ALTER TABLE ... DROP COLUMN IF EXISTS` | Does nothing when the column is not modeled. | +| `ALTER TABLE ... RENAME COLUMN` | Renames the column in its position and in the constraints that list it. | +| `ALTER TABLE ... RENAME TO` | Renames the table in its position, keeping its columns and constraints. | +| `ALTER TABLE ... ALTER COLUMN ... TYPE` | Maps or records the new type as a `CREATE TABLE` column of that type is mapped or recorded, keeping the column's position and its nullability, which a type change does not state. | +| `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, such as `SET DEFAULT` or `ADD CONSTRAINT` | Rejected. | +| `DROP` of anything other than a table | Rejected. | + +One `ALTER TABLE` may state several actions; they are applied in the written +order. Table and column names are matched case-insensitively, as query analysis +looks them up, after their SQL identifier delimiters are removed. + +Outside the `IF [NOT] EXISTS` forms above, a statement that refers to a table or +a column that is not modeled is rejected, and so is a statement that would give +two tables, or two columns of one table, the same name, as PostgreSQL rejects it: + +```text +sqlcj: Invalid schema source /home/dev/project/sql/migrations/V2__orders.sql: Table not found in schema: payments +sqlcj: Invalid schema source /home/dev/project/sql/migrations/V2__orders.sql: Column not found in table orders: total +sqlcj: Invalid schema source /home/dev/project/sql/migrations/V2__orders.sql: Table already exists in schema: orders +sqlcj: Invalid schema source /home/dev/project/sql/migrations/V2__orders.sql: Column already exists in table orders: total +``` + ## Nulls Every generated method parameter and every generated result component uses a @@ -245,12 +289,13 @@ written by an earlier successful run is left unchanged. The writing step itself is sequential rather than atomic; see [Generated Output and Failures](configuration.md#generated-output-and-failures). -A schema message names the schema source and the offending statement, or the -syntax error with the line and column it was found at. A query message names the +A schema message names the schema source and the offending statement, the table +or column the statement refers to, or the syntax error with the line and column +it was found at. A query message names the query, its source, its header line, and the offending column and recorded type: ```text -sqlcj: Invalid schema source /home/dev/project/schema.sql: Unsupported schema statement: Alter +sqlcj: Invalid schema source /home/dev/project/schema.sql: Unsupported schema statement: CreateView 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 VARCHAR[] ``` @@ -298,6 +343,44 @@ Type table: - `PostgresIntegrationTest.shouldExecuteGeneratedQueryForSnapshotWithIgnoredTableConstraints` proves that such a snapshot is valid PostgreSQL DDL and compiles and executes. +Ordered table DDL: + +- `DefaultSchemaParserTest.shouldComposeTheSchemaOfSeveralParsedSources`, + `DefaultSchemaParserTest.shouldAppendAddedColumnsTypedLikeCreateTableColumns`, + `DefaultSchemaParserTest.shouldRecordTheColumnConstraintsOfAnAddedColumn`, + `DefaultSchemaParserTest.shouldDropColumnAndTheConstraintsThatListIt`, + `DefaultSchemaParserTest.shouldRenameColumnInItsPositionAndInItsConstraints`, + `DefaultSchemaParserTest.shouldRenameTableInItsPosition`, + `DefaultSchemaParserTest.shouldChangeColumnTypeInItsPositionKeepingItsNullability`, + `DefaultSchemaParserTest.shouldChangeARecordedColumnTypeBackToAMappedType`, + `DefaultSchemaParserTest.shouldSetAndDropColumnNullability`, + `DefaultSchemaParserTest.shouldApplyTheActionsOfOneAlterTableInOrder`, + `DefaultSchemaParserTest.shouldDropEveryTableOfOneDropStatement`, and + `DefaultSchemaParserTest.shouldMatchTableAndColumnNamesCaseInsensitively` + cover the applied forms. +- `DefaultSchemaParserTest.shouldIgnoreCreateTableIfNotExistsForAnExistingTable`, + `DefaultSchemaParserTest.shouldIgnoreAddColumnIfNotExistsForAnExistingColumn`, + `DefaultSchemaParserTest.shouldIgnoreDropTableIfExistsForAMissingTable`, + `DefaultSchemaParserTest.shouldIgnoreAlterTableIfExistsForAMissingTable`, + `DefaultSchemaParserTest.shouldIgnoreDropColumnIfExistsForAMissingColumn`, and + `DefaultSchemaParserTest.shouldReportTheMissingColumnOfAnAlterTableIfExists` + cover the `IF [NOT] EXISTS` variants. +- `DefaultSchemaParserTest.shouldReportTheMissingTableOfAStatement`, + `DefaultSchemaParserTest.shouldReportTheMissingColumnOfAnAlterTableAction`, + `DefaultSchemaParserTest.shouldReportAStatementThatRepeatsATableName`, + `DefaultSchemaParserTest.shouldReportAStatementThatRepeatsAColumnName`, + `DefaultSchemaParserTest.shouldRejectUnsupportedAlterTableAction`, + `DefaultSchemaParserTest.shouldRejectAlterColumnSetStatistics`, and + `DefaultSchemaParserTest.shouldRejectAlterColumnAddIdentity` 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 + snapshot, and + `SqlcjCompilerIntegrationTest.shouldReportTheMigrationFileThatAltersAMissingTable` + covers the file-naming diagnostic of a missing reference and that no file is + written. + Nulls: - `PostgresIntegrationTest.shouldBindAndReadNullValuesThroughGeneratedCode`