From 9c2e6d24dc0acb399fb17bd433a3f09c64fefa96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 16 Oct 2024 16:15:49 +0200 Subject: [PATCH 01/64] support table references in foreign key definitions --- docs/README.md | 2 +- ...oreign_key_table_reference_constraint.json | 18 ++ pkg/migrations/errors.go | 8 + pkg/migrations/fk_reference.go | 14 +- pkg/migrations/op_add_column_test.go | 8 +- pkg/migrations/op_alter_column_test.go | 2 +- pkg/migrations/op_change_type_test.go | 2 +- pkg/migrations/op_create_table.go | 12 +- pkg/migrations/op_create_table_test.go | 6 +- pkg/migrations/op_drop_constraint_test.go | 4 +- pkg/migrations/op_drop_not_null_test.go | 2 +- pkg/migrations/op_set_check_test.go | 2 +- pkg/migrations/op_set_fk.go | 12 +- pkg/migrations/op_set_fk_test.go | 202 ++++++++++++++++-- pkg/migrations/op_set_notnull_test.go | 2 +- pkg/migrations/op_set_unique_test.go | 2 +- pkg/migrations/types.go | 2 +- schema.json | 2 +- 18 files changed, 256 insertions(+), 46 deletions(-) create mode 100644 examples/38_add_foreign_key_table_reference_constraint.json diff --git a/docs/README.md b/docs/README.md index 666a4f5d..ad8d648d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -870,7 +870,7 @@ Add foreign key operations add a foreign key constraint to a column. "references": { "name": "name of foreign key reference", "table": "name of referenced table", - "column": "name of referenced column", + "column": "name of referenced column (optional)", "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", }, "up": "SQL expression", diff --git a/examples/38_add_foreign_key_table_reference_constraint.json b/examples/38_add_foreign_key_table_reference_constraint.json new file mode 100644 index 00000000..f97e83cd --- /dev/null +++ b/examples/38_add_foreign_key_table_reference_constraint.json @@ -0,0 +1,18 @@ +{ + "name": "38_add_foreign_key_table_reference_constraint", + "operations": [ + { + "alter_column": { + "table": "posts", + "column": "user_id", + "references": { + "name": "fk_users_id", + "table": "users", + "on_delete": "CASCADE" + }, + "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", + "down": "user_id" + } + } + ] +} diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index f0232848..8428bcea 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -111,6 +111,14 @@ func (e ColumnReferenceError) Error() string { e.Err.Error()) } +type PrimaryKeyDoesNotExistError struct { + Table string +} + +func (e PrimaryKeyDoesNotExistError) Error() string { + return fmt.Sprintf("table %q does not have a primary key", e.Table) +} + type CheckConstraintError struct { Table string Column string diff --git a/pkg/migrations/fk_reference.go b/pkg/migrations/fk_reference.go index 7cbd72d8..659b4a42 100644 --- a/pkg/migrations/fk_reference.go +++ b/pkg/migrations/fk_reference.go @@ -19,9 +19,17 @@ func (f *ForeignKeyReference) Validate(s *schema.Schema) error { return TableDoesNotExistError{Name: f.Table} } - column := table.GetColumn(f.Column) - if column == nil { - return ColumnDoesNotExistError{Table: f.Table, Name: f.Column} + if f.Column != nil { + // check if column exists in case of column reference + column := table.GetColumn(*f.Column) + if column == nil { + return ColumnDoesNotExistError{Table: f.Table, Name: *f.Column} + } + } else { + // check if primary key exists in case of table reference + if len(table.PrimaryKey) == 0 { + return PrimaryKeyDoesNotExistError{Table: f.Table} + } } switch strings.ToUpper(string(f.OnDelete)) { diff --git a/pkg/migrations/op_add_column_test.go b/pkg/migrations/op_add_column_test.go index 0a248053..320001c5 100644 --- a/pkg/migrations/op_add_column_test.go +++ b/pkg/migrations/op_add_column_test.go @@ -205,7 +205,7 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Nullable: ptr(true), }, @@ -307,7 +307,7 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Nullable: ptr(false), }, @@ -410,7 +410,7 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, }, }, @@ -521,7 +521,7 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: "CASCADE", }, }, diff --git a/pkg/migrations/op_alter_column_test.go b/pkg/migrations/op_alter_column_test.go index c8192ce4..f4a76291 100644 --- a/pkg/migrations/op_alter_column_test.go +++ b/pkg/migrations/op_alter_column_test.go @@ -414,7 +414,7 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) { Nullable: ptr(false), References: &migrations.ForeignKeyReference{ Table: "events", - Column: "id", + Column: ptr("id"), Name: "person_manages_event_fk", }, }, diff --git a/pkg/migrations/op_change_type_test.go b/pkg/migrations/op_change_type_test.go index bf36e718..bf0ec9de 100644 --- a/pkg/migrations/op_change_type_test.go +++ b/pkg/migrations/op_change_type_test.go @@ -192,7 +192,7 @@ func TestChangeColumnType(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 5e4520b2..055916cb 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -152,10 +152,16 @@ func ColumnToSQL(col Column, tr SQLTransformer) (string, error) { onDelete = strings.ToUpper(string(col.References.OnDelete)) } - sql += fmt.Sprintf(" CONSTRAINT %s REFERENCES %s(%s) ON DELETE %s", + var references string + if col.References.Column != nil { + references = fmt.Sprintf("%s(%s)", pq.QuoteIdentifier(col.References.Table), pq.QuoteIdentifier(*col.References.Column)) + } else { + references = pq.QuoteIdentifier(col.References.Table) + } + + sql += fmt.Sprintf(" CONSTRAINT %s REFERENCES %s ON DELETE %s", pq.QuoteIdentifier(col.References.Name), - pq.QuoteIdentifier(col.References.Table), - pq.QuoteIdentifier(col.References.Column), + references, onDelete) } if col.Check != nil { diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 85de4713..769e8590 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -114,7 +114,7 @@ func TestCreateTable(t *testing.T) { Name: "user_id", Type: "integer", References: &migrations.ForeignKeyReference{ - Column: "id", + Column: ptr("id"), Name: "fk_users_id", Table: "users", }, @@ -221,7 +221,7 @@ func TestCreateTable(t *testing.T) { Name: "user_id", Type: "integer", References: &migrations.ForeignKeyReference{ - Column: "id", + Column: ptr("id"), Name: "fk_users_id", Table: "users", OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, @@ -447,7 +447,7 @@ func TestCreateTableValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_doesntexist", Table: "users", - Column: "doesntexist", + Column: ptr("doesntexist"), }, }, { diff --git a/pkg/migrations/op_drop_constraint_test.go b/pkg/migrations/op_drop_constraint_test.go index da58cafa..22fbad80 100644 --- a/pkg/migrations/op_drop_constraint_test.go +++ b/pkg/migrations/op_drop_constraint_test.go @@ -260,7 +260,7 @@ func TestDropConstraint(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -572,7 +572,7 @@ func TestDropConstraint(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: "id", + Column: ptr("id"), }, }, }, diff --git a/pkg/migrations/op_drop_not_null_test.go b/pkg/migrations/op_drop_not_null_test.go index 446a7adf..bd22db21 100644 --- a/pkg/migrations/op_drop_not_null_test.go +++ b/pkg/migrations/op_drop_not_null_test.go @@ -263,7 +263,7 @@ func TestDropNotNull(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: "id", + Column: ptr("id"), }, }, }, diff --git a/pkg/migrations/op_set_check_test.go b/pkg/migrations/op_set_check_test.go index 602fc806..7b063c4c 100644 --- a/pkg/migrations/op_set_check_test.go +++ b/pkg/migrations/op_set_check_test.go @@ -260,7 +260,7 @@ func TestSetCheckConstraint(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, diff --git a/pkg/migrations/op_set_fk.go b/pkg/migrations/op_set_fk.go index 8f166a96..b85eef01 100644 --- a/pkg/migrations/op_set_fk.go +++ b/pkg/migrations/op_set_fk.go @@ -77,13 +77,19 @@ func (o *OpSetForeignKey) addForeignKeyConstraint(ctx context.Context, conn db.D onDelete = strings.ToUpper(string(o.References.OnDelete)) } + var references string + if o.References.Column != nil { + references = fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), pq.QuoteIdentifier(*o.References.Column)) + } else { + references = pq.QuoteIdentifier(o.References.Table) + } + _, err := conn.ExecContext(ctx, - fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s NOT VALID", + fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s ON DELETE %s NOT VALID", pq.QuoteIdentifier(o.Table), pq.QuoteIdentifier(o.References.Name), pq.QuoteIdentifier(tempColumnName), - pq.QuoteIdentifier(o.References.Table), - pq.QuoteIdentifier(o.References.Column), + references, onDelete, )) diff --git a/pkg/migrations/op_set_fk_test.go b/pkg/migrations/op_set_fk_test.go index 665048ed..5c4305aa 100644 --- a/pkg/migrations/op_set_fk_test.go +++ b/pkg/migrations/op_set_fk_test.go @@ -16,7 +16,7 @@ func TestSetForeignKey(t *testing.T) { ExecuteTests(t, TestCases{ { - name: "add foreign key constraint", + name: "add foreign key constraint with column reference", migrations: []migrations.Migration{ { Name: "01_add_tables", @@ -65,7 +65,171 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), + }, + Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", + Down: "user_id", + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The new (temporary) `user_id` column should exist on the underlying table. + ColumnMustExist(t, db, schema, "posts", migrations.TemporaryName("user_id")) + + // A temporary FK constraint has been created on the temporary column + NotValidatedForeignKeyMustExist(t, db, schema, "posts", "fk_users_id") + + // Inserting some data into the `users` table works. + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "alice", + }) + MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ + "name": "bob", + }) + + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by alice", + "user_id": "1", + }) + + // Inserting data into the new `posts` view with an invalid user reference fails. + MustNotInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by unknown user", + "user_id": "3", + }, testutils.FKViolationErrorCode) + + // The post that was inserted successfully has been backfilled into the old view. + rows := MustSelect(t, db, schema, "01_add_tables", "posts") + assert.Equal(t, []map[string]any{ + {"id": 1, "title": "post by alice", "user_id": 1}, + }, rows) + + // Inserting data into the old `posts` view with a valid user reference works. + MustInsert(t, db, schema, "01_add_tables", "posts", map[string]string{ + "title": "post by bob", + "user_id": "2", + }) + + // Inserting data into the old `posts` view with an invalid user reference also works. + MustInsert(t, db, schema, "01_add_tables", "posts", map[string]string{ + "title": "post by unknown user", + "user_id": "3", + }) + + // The post that was inserted successfully has been backfilled into the new view. + // The post by an unknown user has been backfilled with a NULL user_id. + rows = MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Equal(t, []map[string]any{ + {"id": 1, "title": "post by alice", "user_id": 1}, + {"id": 3, "title": "post by bob", "user_id": 2}, + {"id": 4, "title": "post by unknown user", "user_id": nil}, + }, rows) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The new (temporary) `user_id` column should not exist on the underlying table. + ColumnMustNotExist(t, db, schema, "posts", migrations.TemporaryName("user_id")) + + // The up function no longer exists. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", "user_id")) + // The down function no longer exists. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", migrations.TemporaryName("user_id"))) + + // The up trigger no longer exists. + TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", "user_id")) + // The down trigger no longer exists. + TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", migrations.TemporaryName("user_id"))) + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The new (temporary) `user_id` column should not exist on the underlying table. + ColumnMustNotExist(t, db, schema, "posts", migrations.TemporaryName("user_id")) + + // A validated foreign key constraint exists on the underlying table. + ValidatedForeignKeyMustExist(t, db, schema, "posts", "fk_users_id") + + // Inserting data into the new `posts` view with a valid user reference works. + MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "another post by alice", + "user_id": "1", + }) + + // Inserting data into the new `posts` view with an invalid user reference fails. + MustNotInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ + "title": "post by unknown user", + "user_id": "3", + }, testutils.FKViolationErrorCode) + + // The data in the new `posts` view is as expected. + rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") + assert.Equal(t, []map[string]any{ + {"id": 1, "title": "post by alice", "user_id": 1}, + {"id": 3, "title": "post by bob", "user_id": 2}, + {"id": 4, "title": "post by unknown user", "user_id": nil}, + {"id": 5, "title": "another post by alice", "user_id": 1}, + }, rows) + + // The up function no longer exists. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", "user_id")) + // The down function no longer exists. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", migrations.TemporaryName("user_id"))) + + // The up trigger no longer exists. + TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", "user_id")) + // The down trigger no longer exists. + TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", migrations.TemporaryName("user_id"))) + }, + }, + { + name: "add foreign key constraint with table reference", + migrations: []migrations.Migration{ + { + Name: "01_add_tables", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + }, + }, + }, + &migrations.OpCreateTable{ + Name: "posts", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "title", + Type: "text", + }, + { + Name: "user_id", + Type: "integer", + Nullable: ptr(true), + }, + }, + }, + }, + }, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_id", + Table: "users", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -230,7 +394,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -319,7 +483,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", @@ -425,7 +589,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", @@ -535,7 +699,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteSETDEFAULT, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", @@ -647,7 +811,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -740,7 +904,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id_1", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", @@ -757,7 +921,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id_2", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -829,7 +993,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -912,7 +1076,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -998,7 +1162,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1100,7 +1264,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1176,7 +1340,7 @@ func TestSetForeignKeyValidation(t *testing.T) { Column: "user_id", References: &migrations.ForeignKeyReference{ Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1203,7 +1367,7 @@ func TestSetForeignKeyValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_doesntexist_id", Table: "doesntexist", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1230,7 +1394,7 @@ func TestSetForeignKeyValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_doesntexist", Table: "users", - Column: "doesntexist", + Column: ptr("doesntexist"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1257,7 +1421,7 @@ func TestSetForeignKeyValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_doesntexist", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: "invalid", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", @@ -1284,7 +1448,7 @@ func TestSetForeignKeyValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_doesntexist", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: "no action", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", @@ -1308,7 +1472,7 @@ func TestSetForeignKeyValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_doesntexist", Table: "users", - Column: "id", + Column: ptr("id"), OnDelete: "SET NULL", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", diff --git a/pkg/migrations/op_set_notnull_test.go b/pkg/migrations/op_set_notnull_test.go index 87cc3390..ebb38361 100644 --- a/pkg/migrations/op_set_notnull_test.go +++ b/pkg/migrations/op_set_notnull_test.go @@ -269,7 +269,7 @@ func TestSetNotNull(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, diff --git a/pkg/migrations/op_set_unique_test.go b/pkg/migrations/op_set_unique_test.go index 2b05a519..89516ca4 100644 --- a/pkg/migrations/op_set_unique_test.go +++ b/pkg/migrations/op_set_unique_test.go @@ -306,7 +306,7 @@ func TestSetColumnUnique(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: "id", + Column: ptr("id"), OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, }, }, diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 0e05e8c2..af14bab6 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -47,7 +47,7 @@ type Column struct { // Foreign key reference definition type ForeignKeyReference struct { // Name of the referenced column - Column string `json:"column"` + Column *string `json:"column,omitempty"` // Name of the foreign key constraint Name string `json:"name"` diff --git a/schema.json b/schema.json index b1f24c52..87ac9056 100644 --- a/schema.json +++ b/schema.json @@ -88,7 +88,7 @@ "default": "NO ACTION" } }, - "required": ["column", "name", "table"], + "required": ["name", "table"], "type": "object" }, "OpAddColumn": { From 88ee25b9d51c4d9506d6efac6873a7dc64ba8fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 17 Oct 2024 15:36:24 +0200 Subject: [PATCH 02/64] in progress --- examples/01_create_tables.json | 22 ++++++ ...oreign_key_table_reference_constraint.json | 21 +++++- pkg/migrations/fk_reference.go | 26 +++++-- pkg/migrations/op_add_column.go | 67 ++++++++++++++++++- pkg/migrations/op_create_table.go | 52 +++----------- pkg/migrations/types.go | 3 + schema.json | 8 +++ 7 files changed, 149 insertions(+), 50 deletions(-) diff --git a/examples/01_create_tables.json b/examples/01_create_tables.json index 1bd018e2..0548f533 100644 --- a/examples/01_create_tables.json +++ b/examples/01_create_tables.json @@ -42,6 +42,28 @@ } ] } + }, + { + "create_table": { + "name": "sellers", + "columns": [ + { + "name": "name", + "type": "varchar(255)", + "pk": true + }, + { + "name": "zip", + "type": "integer", + "pk": true + }, + { + "name": "description", + "type": "varchar(255)", + "nullable": true + } + ] + } } ] } diff --git a/examples/38_add_foreign_key_table_reference_constraint.json b/examples/38_add_foreign_key_table_reference_constraint.json index f97e83cd..80ea2b24 100644 --- a/examples/38_add_foreign_key_table_reference_constraint.json +++ b/examples/38_add_foreign_key_table_reference_constraint.json @@ -6,13 +6,30 @@ "table": "posts", "column": "user_id", "references": { - "name": "fk_users_id", + "name": "fk_users_table", "table": "users", "on_delete": "CASCADE" }, "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", "down": "user_id" } - } + }, + { + "create_table": { + "name": "tickets", + "columns": [ + { + "name": "sellers_ref", + "nullable": true, + "references": { + "name": "fk_sellers", + "table": "sellers", + "columns": ["name", "zip"], + "on_delete": "CASCADE" + } + } + ] + } + } ] } diff --git a/pkg/migrations/fk_reference.go b/pkg/migrations/fk_reference.go index 659b4a42..d516d502 100644 --- a/pkg/migrations/fk_reference.go +++ b/pkg/migrations/fk_reference.go @@ -3,6 +3,7 @@ package migrations import ( + "fmt" "strings" "github.com/xataio/pgroll/pkg/schema" @@ -14,21 +15,38 @@ func (f *ForeignKeyReference) Validate(s *schema.Schema) error { return FieldRequiredError{Name: "name"} } + if f.Columns != nil && f.Column != nil { + return fmt.Errorf("only one of column or columns is allowed") + } + table := s.GetTable(f.Table) if table == nil { return TableDoesNotExistError{Name: f.Table} } + // check if table reference + if f.Columns != nil && len(f.Columns) == 0 { + // check if primary key exists in case of table reference + if len(table.PrimaryKey) == 0 { + return PrimaryKeyDoesNotExistError{Table: f.Table} + } + } + // check if single column reference if f.Column != nil { // check if column exists in case of column reference column := table.GetColumn(*f.Column) if column == nil { return ColumnDoesNotExistError{Table: f.Table, Name: *f.Column} } - } else { - // check if primary key exists in case of table reference - if len(table.PrimaryKey) == 0 { - return PrimaryKeyDoesNotExistError{Table: f.Table} + } + // check if multiple column reference + if f.Columns != nil { + for _, col := range f.Columns { + // check if column exists in case of column reference + column := table.GetColumn(col) + if column == nil { + return ColumnDoesNotExistError{Table: f.Table, Name: col} + } } } diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index 02a2fb55..88add0ac 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -203,11 +203,15 @@ func addColumn(ctx context.Context, conn db.DB, o OpAddColumn, t *schema.Table, o.Column.Check = nil o.Column.Name = TemporaryName(o.Column.Name) - colSQL, err := ColumnToSQL(o.Column, tr) + columnWriter := ColumnSQLWriter{WithPK: true, Transformer: tr} + colSQL, err := columnWriter.Write(o.Column) + fmt.Println(colSQL) if err != nil { return err } + fmt.Printf("ALTER TABLE %s ADD COLUMN %s\n", pq.QuoteIdentifier(t.Name), colSQL) + _, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s", pq.QuoteIdentifier(t.Name), colSQL, @@ -243,3 +247,64 @@ func NotNullConstraintName(columnName string) string { func IsNotNullConstraintName(name string) bool { return strings.HasPrefix(name, "_pgroll_check_not_null_") } + +// ColumnSQLWriter writes a column to SQL +// It can optionally include the primary key constraint +type ColumnSQLWriter struct { + WithPK bool + WithFKConstraint bool + Transformer SQLTransformer +} + +func (w ColumnSQLWriter) Write(col Column) (string, error) { + sql := fmt.Sprintf("%s %s", pq.QuoteIdentifier(col.Name), col.Type) + + if w.WithPK && col.IsPrimaryKey() { + sql += " PRIMARY KEY" + } + + if col.IsUnique() { + sql += " UNIQUE" + } + if !col.IsNullable() { + sql += " NOT NULL" + } + if col.Default != nil { + d, err := w.Transformer.TransformSQL(*col.Default) + if err != nil { + return "", err + } + sql += fmt.Sprintf(" DEFAULT %s", d) + } + if col.References != nil { + onDelete := "NO ACTION" + if col.References.OnDelete != "" { + onDelete = strings.ToUpper(string(col.References.OnDelete)) + } + var fkName, references string + if col.References.Column != nil { + fkName = pq.QuoteIdentifier(col.References.Name) + references = fmt.Sprintf("%s(%s)", pq.QuoteIdentifier(col.References.Table), pq.QuoteIdentifier(*col.References.Column)) + } + if col.References.Columns != nil { + quotedCols := make([]string, len(col.References.Columns)) + for i, c := range col.References.Columns { + quotedCols[i] = pq.QuoteIdentifier(c) + } + fkName = "FOREIGN KEY " + pq.QuoteIdentifier(col.References.Name) + references = fmt.Sprintf("%s(%s)", pq.QuoteIdentifier(col.References.Table), strings.Join(quotedCols, ", ")) + } + + sql += fmt.Sprintf(" CONSTRAINT %s REFERENCES %s ON DELETE %s", + fkName, + references, + onDelete) + } + if col.Check != nil { + sql += fmt.Sprintf(" CONSTRAINT %s CHECK (%s)", + pq.QuoteIdentifier(col.Check.Name), + col.Check.Constraint) + } + fmt.Println(sql) + return sql, nil +} diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 055916cb..631e014d 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -23,6 +23,7 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri // Create the table under a temporary name tempName := TemporaryName(o.Name) + fmt.Printf("CREATE TABLE %s (%s)\n", tempName, columnsSQL) _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s)", pq.QuoteIdentifier(tempName), columnsSQL)) @@ -113,61 +114,26 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { func columnsToSQL(cols []Column, tr SQLTransformer) (string, error) { var sql string + var primaryKeys []string + columnWriter := ColumnSQLWriter{WithPK: false, Transformer: tr} for i, col := range cols { if i > 0 { sql += ", " } - colSQL, err := ColumnToSQL(col, tr) + colSQL, err := columnWriter.Write(col) if err != nil { return "", err } sql += colSQL - } - return sql, nil -} - -// ColumnToSQL generates the SQL for a column definition. -func ColumnToSQL(col Column, tr SQLTransformer) (string, error) { - sql := fmt.Sprintf("%s %s", pq.QuoteIdentifier(col.Name), col.Type) - if col.IsPrimaryKey() { - sql += " PRIMARY KEY" - } - if col.IsUnique() { - sql += " UNIQUE" - } - if !col.IsNullable() { - sql += " NOT NULL" - } - if col.Default != nil { - d, err := tr.TransformSQL(*col.Default) - if err != nil { - return "", err + if col.IsPrimaryKey() { + primaryKeys = append(primaryKeys, pq.QuoteIdentifier(col.Name)) } - sql += fmt.Sprintf(" DEFAULT %s", d) } - if col.References != nil { - onDelete := "NO ACTION" - if col.References.OnDelete != "" { - onDelete = strings.ToUpper(string(col.References.OnDelete)) - } - var references string - if col.References.Column != nil { - references = fmt.Sprintf("%s(%s)", pq.QuoteIdentifier(col.References.Table), pq.QuoteIdentifier(*col.References.Column)) - } else { - references = pq.QuoteIdentifier(col.References.Table) - } - - sql += fmt.Sprintf(" CONSTRAINT %s REFERENCES %s ON DELETE %s", - pq.QuoteIdentifier(col.References.Name), - references, - onDelete) - } - if col.Check != nil { - sql += fmt.Sprintf(" CONSTRAINT %s CHECK (%s)", - pq.QuoteIdentifier(col.Check.Name), - col.Check.Constraint) + if len(primaryKeys) > 0 { + sql += fmt.Sprintf(", PRIMARY KEY (%s)", strings.Join(primaryKeys, ", ")) } + return sql, nil } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index af14bab6..21f8d652 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -49,6 +49,9 @@ type ForeignKeyReference struct { // Name of the referenced column Column *string `json:"column,omitempty"` + // List of referenced columns + Columns []string `json:"columns,omitempty"` + // Name of the foreign key constraint Name string `json:"name"` diff --git a/schema.json b/schema.json index 87ac9056..94367536 100644 --- a/schema.json +++ b/schema.json @@ -73,6 +73,13 @@ "description": "Name of the referenced column", "type": "string" }, + "columns": { + "description": "List of referenced columns", + "items": { + "type": "string" + }, + "type": "array" + }, "name": { "description": "Name of the foreign key constraint", "type": "string" @@ -89,6 +96,7 @@ } }, "required": ["name", "table"], + "anyOf": [{ "required": ["column"] }, { "required": ["columns"] }], "type": "object" }, "OpAddColumn": { From 4b36f1f909ada3b6e01d0510a912fdfc67df2033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 17 Oct 2024 17:14:25 +0200 Subject: [PATCH 03/64] more --- ...oreign_key_table_reference_constraint.json | 35 -------------- examples/38_create_tickets_table.json | 29 ++++++++++++ ...oreign_key_table_reference_constraint.json | 47 +++++++++++++++++++ pkg/migrations/op_add_column.go | 4 +- 4 files changed, 78 insertions(+), 37 deletions(-) delete mode 100644 examples/38_add_foreign_key_table_reference_constraint.json create mode 100644 examples/38_create_tickets_table.json create mode 100644 examples/39_add_foreign_key_table_reference_constraint.json diff --git a/examples/38_add_foreign_key_table_reference_constraint.json b/examples/38_add_foreign_key_table_reference_constraint.json deleted file mode 100644 index 80ea2b24..00000000 --- a/examples/38_add_foreign_key_table_reference_constraint.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "name": "38_add_foreign_key_table_reference_constraint", - "operations": [ - { - "alter_column": { - "table": "posts", - "column": "user_id", - "references": { - "name": "fk_users_table", - "table": "users", - "on_delete": "CASCADE" - }, - "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", - "down": "user_id" - } - }, - { - "create_table": { - "name": "tickets", - "columns": [ - { - "name": "sellers_ref", - "nullable": true, - "references": { - "name": "fk_sellers", - "table": "sellers", - "columns": ["name", "zip"], - "on_delete": "CASCADE" - } - } - ] - } - } - ] -} diff --git a/examples/38_create_tickets_table.json b/examples/38_create_tickets_table.json new file mode 100644 index 00000000..29521756 --- /dev/null +++ b/examples/38_create_tickets_table.json @@ -0,0 +1,29 @@ +{ + "name": "38_create_tickets_table", + "operations": [ + { + "create_table": { + "name": "tickets", + "columns": [ + { + "name": "sellers_name", + "type": "varchar(255)" + }, + { + "name": "sellers_zip", + "type": "integer" + }, + { + "name": "event_id", + "type": "integer", + "references": { + "name": "fk_events", + "table": "events", + "on_delete": "CASCADE" + } + } + ] + } + } + ] +} diff --git a/examples/39_add_foreign_key_table_reference_constraint.json b/examples/39_add_foreign_key_table_reference_constraint.json new file mode 100644 index 00000000..daf232a5 --- /dev/null +++ b/examples/39_add_foreign_key_table_reference_constraint.json @@ -0,0 +1,47 @@ +{ + "name": "39_add_foreign_key_table_reference_constraint", + "operations": [ + { + "alter_column": { + "table": "tickets", + "column": "sellers_name", + "references": { + "name": "fk_sellers", + "table": "sellers", + "column": "name", + "on_delete": "CASCADE" + }, + "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM sellers WHERE sellers.name = sellers_name) THEN sellers_name ELSE NULL END)", + "down": "sellers_name" + } + }, + { + "alter_column": { + "table": "tickets", + "column": "sellers_zip", + "references": { + "name": "fk_events", + "table": "sellers", + "column": "zip", + "on_delete": "CASCADE" + }, + "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM sellers WHERE sellers.zip = sellers_zip) THEN sellers_zip ELSE NULL END)", + "down": "sellers_zip" + } + }, + { + "alter_column": { + "table": "tickets", + "column": "event_id", + "references": { + "name": "fk_events", + "table": "events", + "column": "id", + "on_delete": "CASCADE" + }, + "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM events WHERE event.id = event_id) THEN event_id ELSE NULL END)", + "down": "event_id" + } + } + ] +} diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index 88add0ac..2a57b926 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -281,9 +281,9 @@ func (w ColumnSQLWriter) Write(col Column) (string, error) { if col.References.OnDelete != "" { onDelete = strings.ToUpper(string(col.References.OnDelete)) } - var fkName, references string + fkName := pq.QuoteIdentifier(col.References.Name) + references := pq.QuoteIdentifier(col.References.Table) if col.References.Column != nil { - fkName = pq.QuoteIdentifier(col.References.Name) references = fmt.Sprintf("%s(%s)", pq.QuoteIdentifier(col.References.Table), pq.QuoteIdentifier(*col.References.Column)) } if col.References.Columns != nil { From aa4279e2659fdcb49a50d0e4b8e46cc1f7b21a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 25 Oct 2024 12:53:47 +0200 Subject: [PATCH 04/64] rename migration files --- ...8_create_tickets_table.json => 39_create_tickets_table.json} | 2 +- ....json => 40_add_foreign_key_table_reference_constraint.json} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename examples/{38_create_tickets_table.json => 39_create_tickets_table.json} (93%) rename examples/{39_add_foreign_key_table_reference_constraint.json => 40_add_foreign_key_table_reference_constraint.json} (95%) diff --git a/examples/38_create_tickets_table.json b/examples/39_create_tickets_table.json similarity index 93% rename from examples/38_create_tickets_table.json rename to examples/39_create_tickets_table.json index 29521756..89070c73 100644 --- a/examples/38_create_tickets_table.json +++ b/examples/39_create_tickets_table.json @@ -1,5 +1,5 @@ { - "name": "38_create_tickets_table", + "name": "39_create_tickets_table", "operations": [ { "create_table": { diff --git a/examples/39_add_foreign_key_table_reference_constraint.json b/examples/40_add_foreign_key_table_reference_constraint.json similarity index 95% rename from examples/39_add_foreign_key_table_reference_constraint.json rename to examples/40_add_foreign_key_table_reference_constraint.json index daf232a5..4c7ac531 100644 --- a/examples/39_add_foreign_key_table_reference_constraint.json +++ b/examples/40_add_foreign_key_table_reference_constraint.json @@ -1,5 +1,5 @@ { - "name": "39_add_foreign_key_table_reference_constraint", + "name": "40_add_foreign_key_table_reference_constraint", "operations": [ { "alter_column": { From d6afb8d346f4bf900bf98d8d026335c86beef50a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 25 Oct 2024 14:01:06 +0200 Subject: [PATCH 05/64] minifix --- pkg/migrations/op_set_fk_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/migrations/op_set_fk_test.go b/pkg/migrations/op_set_fk_test.go index 063d6293..a3433b1d 100644 --- a/pkg/migrations/op_set_fk_test.go +++ b/pkg/migrations/op_set_fk_test.go @@ -1334,7 +1334,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1350,7 +1350,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: "id", + Column: ptr("id"), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", From 1fdab428dfb49b2908dd10adde1d9b794e141380 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 30 Oct 2024 16:02:38 +0100 Subject: [PATCH 06/64] more changes --- examples/39_create_tickets_table.json | 5 ++ ...oreign_key_table_reference_constraint.json | 16 +------ ...add_column_with_multiple_pk_in_table.json} | 0 pkg/migrations/types.go | 27 +++++++++++ schema.json | 47 +++++++++++++++++++ 5 files changed, 80 insertions(+), 15 deletions(-) rename examples/{39_add_column_with_multiple_pk_in_table.json => 41_add_column_with_multiple_pk_in_table.json} (100%) diff --git a/examples/39_create_tickets_table.json b/examples/39_create_tickets_table.json index 89070c73..1d617d09 100644 --- a/examples/39_create_tickets_table.json +++ b/examples/39_create_tickets_table.json @@ -5,6 +5,11 @@ "create_table": { "name": "tickets", "columns": [ + { + "name": "ticket_id", + "type": "serial", + "pk": true + }, { "name": "sellers_name", "type": "varchar(255)" diff --git a/examples/40_add_foreign_key_table_reference_constraint.json b/examples/40_add_foreign_key_table_reference_constraint.json index 4c7ac531..1ef86d76 100644 --- a/examples/40_add_foreign_key_table_reference_constraint.json +++ b/examples/40_add_foreign_key_table_reference_constraint.json @@ -20,7 +20,7 @@ "table": "tickets", "column": "sellers_zip", "references": { - "name": "fk_events", + "name": "fk_sellers", "table": "sellers", "column": "zip", "on_delete": "CASCADE" @@ -28,20 +28,6 @@ "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM sellers WHERE sellers.zip = sellers_zip) THEN sellers_zip ELSE NULL END)", "down": "sellers_zip" } - }, - { - "alter_column": { - "table": "tickets", - "column": "event_id", - "references": { - "name": "fk_events", - "table": "events", - "column": "id", - "on_delete": "CASCADE" - }, - "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM events WHERE event.id = event_id) THEN event_id ELSE NULL END)", - "down": "event_id" - } } ] } diff --git a/examples/39_add_column_with_multiple_pk_in_table.json b/examples/41_add_column_with_multiple_pk_in_table.json similarity index 100% rename from examples/39_add_column_with_multiple_pk_in_table.json rename to examples/41_add_column_with_multiple_pk_in_table.json diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index fcf82559..ebd43385 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -122,6 +122,33 @@ type OpAlterColumn struct { Up string `json:"up,omitempty"` } +// Add constraint to table operation +type OpCreateConstraint struct { + // Check constraint for table + Check *string `json:"check,omitempty"` + + // Columns to add constraint to + Columns []string `json:"columns,omitempty"` + + // Name of the constraint + Name string `json:"name"` + + // Foreign key constraint for the column + References *ForeignKeyReference `json:"references,omitempty"` + + // Name of the table + Table string `json:"table"` + + // Type of the constraint + Type OpCreateConstraintType `json:"type"` +} + +type OpCreateConstraintType string + +const OpCreateConstraintTypeCheck OpCreateConstraintType = "check" +const OpCreateConstraintTypeForeign OpCreateConstraintType = "foreign" +const OpCreateConstraintTypeUnique OpCreateConstraintType = "unique" + // Create index operation type OpCreateIndex struct { // Names of columns on which to define the index diff --git a/schema.json b/schema.json index 7adbc2b2..5ec694fa 100644 --- a/schema.json +++ b/schema.json @@ -440,6 +440,42 @@ "required": ["identity", "table"], "type": "object" }, + "OpCreateConstraint": { + "additionalProperties": false, + "description": "Add constraint to table operation", + "properties": { + "table": { + "description": "Name of the table", + "type": "string" + }, + "name": { + "description": "Name of the constraint", + "type": "string" + }, + "columns": { + "description": "Columns to add constraint to", + "type": "array", + "items": { + "type": "string" + } + }, + "type": { + "description": "Type of the constraint", + "type": "string", + "enum": ["check", "unique", "foreign"] + }, + "check": { + "type": "string", + "description": "Check constraint for table" + }, + "references": { + "$ref": "#/$defs/ForeignKeyReference", + "description": "Foreign key constraint for the column" + } + }, + "required": ["name", "table", "type"], + "type": "object" + }, "PgRollOperation": { "anyOf": [ { @@ -573,6 +609,17 @@ } }, "required": ["set_replica_identity"] + }, + { + "type": "object", + "description": "Add constraint operation", + "additionalProperties": false, + "properties": { + "create_constraint": { + "$ref": "#/$defs/OpCreateConstraint" + } + }, + "required": ["create_constraint"] } ] }, From 03231e8cb3a3cbbfa2ef26c1304a5db8f819f76e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 30 Oct 2024 17:24:01 +0100 Subject: [PATCH 07/64] moremore --- pkg/migrations/op_add_column.go | 5 - pkg/migrations/op_create_constraint.go | 169 +++++++++++++++++++++++++ pkg/migrations/op_set_check.go | 10 +- pkg/migrations/types.go | 8 +- schema.json | 12 +- 5 files changed, 194 insertions(+), 10 deletions(-) create mode 100644 pkg/migrations/op_create_constraint.go diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index 07ed1d8b..9b2b83ef 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -305,11 +305,6 @@ func (w ColumnSQLWriter) Write(col Column) (string, error) { references, onDelete) - //sql += fmt.Sprintf(" CONSTRAINT %s REFERENCES %s(%s) ON DELETE %s", - // pq.QuoteIdentifier(col.References.Name), - // pq.QuoteIdentifier(col.References.Table), - // pq.QuoteIdentifier(col.References.Column), - // onDelete) } if col.Check != nil { sql += fmt.Sprintf(" CONSTRAINT %s CHECK (%s)", diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go new file mode 100644 index 00000000..fd2bf925 --- /dev/null +++ b/pkg/migrations/op_create_constraint.go @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: Apache-2.0 + +package migrations + +import ( + "context" + "fmt" + "strings" + + "github.com/lib/pq" + + "github.com/xataio/pgroll/pkg/db" + "github.com/xataio/pgroll/pkg/schema" +) + +var _ Operation = (*OpCreateConstraint)(nil) + +func (o *OpCreateConstraint) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) { + table := s.GetTable(o.Table) + + switch o.Type { + case OpCreateConstraintTypeCheck: + return table, o.addCheckConstraint(ctx, conn) + case OpCreateConstraintTypeUnique: + return table, o.addUniqueConstraint(ctx, conn) + case OpCreateConstraintTypeForeignKey: + return table, o.addForeignKeyConstraint(ctx, conn) + } + + return table, nil +} + +func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { + switch o.Type { + case OpCreateConstraintTypeCheck: + checkOp := &OpSetCheckConstraint{ + Table: o.Table, + Check: CheckConstraint{ + Name: o.Name, + }, + } + return checkOp.Complete(ctx, conn, tr, s) + case OpCreateConstraintTypeUnique: + uniqueOp := &OpSetUnique{ + Table: o.Table, + Name: o.Name, + } + return uniqueOp.Complete(ctx, conn, tr, s) + case OpCreateConstraintTypeForeignKey: + fkOp := &OpSetForeignKey{ + Table: o.Table, + References: ForeignKeyReference{ + Name: o.Name, + }, + } + return fkOp.Complete(ctx, conn, tr, s) + } + + return nil +} + +func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error { + return nil +} + +func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) error { + table := s.GetTable(o.Table) + if table == nil { + return TableDoesNotExistError{Name: o.Table} + } + + if err := ValidateIdentifierLength(o.Name); err != nil { + return err + } + + if table.ConstraintExists(o.Name) { + return ConstraintAlreadyExistsError{ + Table: o.Table, + Constraint: o.Name, + } + } + + for _, col := range o.Columns { + if table.GetColumn(col) == nil { + return ColumnDoesNotExistError{ + Table: o.Table, + Name: col, + } + } + } + + switch o.Type { + case OpCreateConstraintTypeCheck: + if o.Check == nil || *o.Check == "" { + return FieldRequiredError{Name: "check"} + } + case OpCreateConstraintTypeUnique: + if len(o.Columns) == 0 { + return FieldRequiredError{Name: "columns"} + } + case OpCreateConstraintTypeForeignKey: + if o.References == nil { + return FieldRequiredError{Name: "references"} + } + if err := o.References.Validate(s); err != nil { + return err + } + } + + return nil +} + +func (o *OpCreateConstraint) addCheckConstraint(ctx context.Context, conn db.DB) error { + _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID", + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(o.Name), + rewriteCheckExpression(*o.Check, o.Columns...), + )) + + return err +} + +func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB) error { + cols := make([]string, len(o.Columns)) + for i, col := range o.Columns { + cols[i] = pq.QuoteIdentifier(TemporaryName(col)) + } + _, err := conn.ExecContext(ctx, fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", + pq.QuoteIdentifier(o.Name), + pq.QuoteIdentifier(o.Table), + strings.Join(cols, ", "), + )) + + return err +} + +func (o *OpCreateConstraint) addForeignKeyConstraint(ctx context.Context, conn db.DB) error { + cols := make([]string, len(o.Columns)) + for i, col := range o.Columns { + cols[i] = pq.QuoteIdentifier(TemporaryName(col)) + } + + onDelete := string(ForeignKeyReferenceOnDeleteNOACTION) + if o.References.OnDelete != "" { + onDelete = strings.ToUpper(string(o.References.OnDelete)) + } + + var references string + if o.References.Column != nil { + references = fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), pq.QuoteIdentifier(*o.References.Column)) + } else if o.References.Columns != nil { + refCols := make([]string, len(o.Columns)) + for i, col := range o.References.Columns { + cols[i] = pq.QuoteIdentifier(TemporaryName(col)) + } + references = fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), strings.Join(refCols, ", ")) + } + + _, err := conn.ExecContext(ctx, + fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s ON DELETE %s NOT VALID", + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(o.Name), + strings.Join(cols, ", "), + references, + onDelete, + )) + + return err +} diff --git a/pkg/migrations/op_set_check.go b/pkg/migrations/op_set_check.go index a0d29e66..cf77f69f 100644 --- a/pkg/migrations/op_set_check.go +++ b/pkg/migrations/op_set_check.go @@ -86,7 +86,7 @@ func (o *OpSetCheckConstraint) addCheckConstraint(ctx context.Context, conn db.D _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID", pq.QuoteIdentifier(o.Table), pq.QuoteIdentifier(o.Check.Name), - rewriteCheckExpression(o.Check.Constraint, o.Column, TemporaryName(o.Column)), + rewriteCheckExpression(o.Check.Constraint, o.Column), )) return err @@ -97,6 +97,10 @@ func (o *OpSetCheckConstraint) addCheckConstraint(ctx context.Context, conn db.D // On migration start, however, the check is actually applied to the new (temporary) // column. // This function naively rewrites the check expression to apply to the new column. -func rewriteCheckExpression(check string, oldColumn, newColumn string) string { - return strings.ReplaceAll(check, oldColumn, newColumn) +func rewriteCheckExpression(check string, oldColumn ...string) string { + newCheck := check + for _, oldColumn := range oldColumn { + newCheck = strings.ReplaceAll(check, oldColumn, TemporaryName(oldColumn)) + } + return newCheck } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index ebd43385..8c04dfaf 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -130,6 +130,9 @@ type OpCreateConstraint struct { // Columns to add constraint to Columns []string `json:"columns,omitempty"` + // SQL expression for down migration + Down string `json:"down,omitempty"` + // Name of the constraint Name string `json:"name"` @@ -141,12 +144,15 @@ type OpCreateConstraint struct { // Type of the constraint Type OpCreateConstraintType `json:"type"` + + // SQL expression for up migration + Up string `json:"up,omitempty"` } type OpCreateConstraintType string const OpCreateConstraintTypeCheck OpCreateConstraintType = "check" -const OpCreateConstraintTypeForeign OpCreateConstraintType = "foreign" +const OpCreateConstraintTypeForeignKey OpCreateConstraintType = "foreign_key" const OpCreateConstraintTypeUnique OpCreateConstraintType = "unique" // Create index operation diff --git a/schema.json b/schema.json index 5ec694fa..956d391d 100644 --- a/schema.json +++ b/schema.json @@ -462,7 +462,7 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["check", "unique", "foreign"] + "enum": ["check", "unique", "foreign_key"] }, "check": { "type": "string", @@ -471,6 +471,16 @@ "references": { "$ref": "#/$defs/ForeignKeyReference", "description": "Foreign key constraint for the column" + }, + "up": { + "default": "", + "description": "SQL expression for up migration", + "type": "string" + }, + "down": { + "default": "", + "description": "SQL expression for down migration", + "type": "string" } }, "required": ["name", "table", "type"], From b84e4b0f5a0d1edd3ee9a6f36d5fc5d727174f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 30 Oct 2024 17:27:18 +0100 Subject: [PATCH 08/64] cleaner --- pkg/migrations/op_set_check.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/migrations/op_set_check.go b/pkg/migrations/op_set_check.go index cf77f69f..657499fa 100644 --- a/pkg/migrations/op_set_check.go +++ b/pkg/migrations/op_set_check.go @@ -98,9 +98,8 @@ func (o *OpSetCheckConstraint) addCheckConstraint(ctx context.Context, conn db.D // column. // This function naively rewrites the check expression to apply to the new column. func rewriteCheckExpression(check string, oldColumn ...string) string { - newCheck := check for _, oldColumn := range oldColumn { - newCheck = strings.ReplaceAll(check, oldColumn, TemporaryName(oldColumn)) + check = strings.ReplaceAll(check, oldColumn, TemporaryName(oldColumn)) } - return newCheck + return check } From 2f354520f58878a33054b4c934b8c874ac2727f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 10:53:15 +0100 Subject: [PATCH 09/64] fix interface --- pkg/migrations/op_create_constraint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index fd2bf925..59676612 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -59,7 +59,7 @@ func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTra return nil } -func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error { +func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { return nil } From ea6452ffa93caa07ce065c67d539afce8936e135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 11:18:05 +0100 Subject: [PATCH 10/64] more --- examples/.ledger | 2 ++ ..._table.json => 39_add_column_with_multiple_pk_in_table.json} | 0 ...9_create_tickets_table.json => 43_create_tickets_table.json} | 0 ....json => 44_add_foreign_key_table_reference_constraint.json} | 0 4 files changed, 2 insertions(+) rename examples/{41_add_column_with_multiple_pk_in_table.json => 39_add_column_with_multiple_pk_in_table.json} (100%) rename examples/{39_create_tickets_table.json => 43_create_tickets_table.json} (100%) rename examples/{40_add_foreign_key_table_reference_constraint.json => 44_add_foreign_key_table_reference_constraint.json} (100%) diff --git a/examples/.ledger b/examples/.ledger index aad357af..f0d13579 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -40,3 +40,5 @@ 40_create_enum_type.json 41_add_enum_column.json 42_create_unique_index.json +43_create_tickets_table.json +44_add_foreign_key_table_reference_constraint.json diff --git a/examples/41_add_column_with_multiple_pk_in_table.json b/examples/39_add_column_with_multiple_pk_in_table.json similarity index 100% rename from examples/41_add_column_with_multiple_pk_in_table.json rename to examples/39_add_column_with_multiple_pk_in_table.json diff --git a/examples/39_create_tickets_table.json b/examples/43_create_tickets_table.json similarity index 100% rename from examples/39_create_tickets_table.json rename to examples/43_create_tickets_table.json diff --git a/examples/40_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json similarity index 100% rename from examples/40_add_foreign_key_table_reference_constraint.json rename to examples/44_add_foreign_key_table_reference_constraint.json From a9ef71a44a3adbb93d90d108c32a5d9ac227d7a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 11:18:55 +0100 Subject: [PATCH 11/64] rename migrations --- examples/43_create_tickets_table.json | 2 +- examples/44_add_foreign_key_table_reference_constraint.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/43_create_tickets_table.json b/examples/43_create_tickets_table.json index 1d617d09..5348e01a 100644 --- a/examples/43_create_tickets_table.json +++ b/examples/43_create_tickets_table.json @@ -1,5 +1,5 @@ { - "name": "39_create_tickets_table", + "name": "43_create_tickets_table", "operations": [ { "create_table": { diff --git a/examples/44_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json index 1ef86d76..88db44b2 100644 --- a/examples/44_add_foreign_key_table_reference_constraint.json +++ b/examples/44_add_foreign_key_table_reference_constraint.json @@ -1,5 +1,5 @@ { - "name": "40_add_foreign_key_table_reference_constraint", + "name": "44_add_foreign_key_table_reference_constraint", "operations": [ { "alter_column": { From e932cf56a4ee72b64e1be63d3c34df68abcfb800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 12:04:59 +0100 Subject: [PATCH 12/64] updae --- ...oreign_key_table_reference_constraint.json | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/examples/44_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json index 88db44b2..85e9aaa1 100644 --- a/examples/44_add_foreign_key_table_reference_constraint.json +++ b/examples/44_add_foreign_key_table_reference_constraint.json @@ -2,11 +2,12 @@ "name": "44_add_foreign_key_table_reference_constraint", "operations": [ { - "alter_column": { + "create_constraint": { + "type": "foreign_key", "table": "tickets", - "column": "sellers_name", + "name": "fk_sellers", + "columns": ["sellers_name", "sellers_name"], "references": { - "name": "fk_sellers", "table": "sellers", "column": "name", "on_delete": "CASCADE" @@ -14,20 +15,6 @@ "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM sellers WHERE sellers.name = sellers_name) THEN sellers_name ELSE NULL END)", "down": "sellers_name" } - }, - { - "alter_column": { - "table": "tickets", - "column": "sellers_zip", - "references": { - "name": "fk_sellers", - "table": "sellers", - "column": "zip", - "on_delete": "CASCADE" - }, - "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM sellers WHERE sellers.zip = sellers_zip) THEN sellers_zip ELSE NULL END)", - "down": "sellers_zip" - } } ] } From 0d5408b300e47650a88f954618191956c5527806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 12:05:36 +0100 Subject: [PATCH 13/64] format --- examples/43_create_tickets_table.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/43_create_tickets_table.json b/examples/43_create_tickets_table.json index 5348e01a..3a1a48f6 100644 --- a/examples/43_create_tickets_table.json +++ b/examples/43_create_tickets_table.json @@ -28,7 +28,7 @@ } } ] - } } + } ] } From 03ab28ac716aa6bb6b5e5a3d40717610ebef8120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 12:08:15 +0100 Subject: [PATCH 14/64] rm debug --- pkg/migrations/op_add_column.go | 1 - pkg/migrations/op_create_table.go | 1 - 2 files changed, 2 deletions(-) diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index d00449e6..f3493b65 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -311,6 +311,5 @@ func (w ColumnSQLWriter) Write(col Column) (string, error) { pq.QuoteIdentifier(col.Check.Name), col.Check.Constraint) } - fmt.Println(sql) return sql, nil } diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 507b2f91..3188f100 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -24,7 +24,6 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri // Create the table under a temporary name tempName := TemporaryName(o.Name) - fmt.Printf("CREATE TABLE %s (%s)\n", tempName, columnsSQL) _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s)", pq.QuoteIdentifier(tempName), columnsSQL)) From 2c4c487f8c8dae5c7f1d14ebaefdc187465e02c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 12:28:38 +0100 Subject: [PATCH 15/64] fix rewrite --- pkg/migrations/duplicate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/migrations/duplicate.go b/pkg/migrations/duplicate.go index c6ce3d07..e02b5c7b 100644 --- a/pkg/migrations/duplicate.go +++ b/pkg/migrations/duplicate.go @@ -136,7 +136,7 @@ func (d *Duplicator) Duplicate(ctx context.Context) error { sql := fmt.Sprintf(cAlterTableAddCheckConstraintSQL, pq.QuoteIdentifier(d.table.Name), pq.QuoteIdentifier(DuplicationName(cc.Name)), - rewriteCheckExpression(cc.Definition, d.column.Name, d.asName), + rewriteCheckExpression(cc.Definition, d.column.Name), ) _, err := d.conn.ExecContext(ctx, sql) From 0ddf44660a69def13bc91d40e9fc1a18d79dbab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 12:32:34 +0100 Subject: [PATCH 16/64] more rewrite fix --- pkg/migrations/op_add_column.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index f3493b65..da2e6281 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -238,7 +238,7 @@ func (o *OpAddColumn) addCheckConstraint(ctx context.Context, conn db.DB) error _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID", pq.QuoteIdentifier(o.Table), pq.QuoteIdentifier(o.Column.Check.Name), - rewriteCheckExpression(o.Column.Check.Constraint, o.Column.Name, TemporaryName(o.Column.Name)), + rewriteCheckExpression(o.Column.Check.Constraint, o.Column.Name), )) return err } From 6f41cb007cd167ef3843bbc05b2845c4a533092c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 13:00:40 +0100 Subject: [PATCH 17/64] add new op for real --- .../44_add_foreign_key_table_reference_constraint.json | 1 + pkg/migrations/op_common.go | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/examples/44_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json index 85e9aaa1..25711657 100644 --- a/examples/44_add_foreign_key_table_reference_constraint.json +++ b/examples/44_add_foreign_key_table_reference_constraint.json @@ -9,6 +9,7 @@ "columns": ["sellers_name", "sellers_name"], "references": { "table": "sellers", + "name": "fk_sellers", "column": "name", "on_delete": "CASCADE" }, diff --git a/pkg/migrations/op_common.go b/pkg/migrations/op_common.go index 364e70f0..5c732fee 100644 --- a/pkg/migrations/op_common.go +++ b/pkg/migrations/op_common.go @@ -24,6 +24,7 @@ const ( OpNameDropConstraint OpName = "drop_constraint" OpNameSetReplicaIdentity OpName = "set_replica_identity" OpRawSQLName OpName = "sql" + OpCreateConstraintName OpName = "create_constraint" // Internal operation types used by `alter_column` OpNameRenameColumn OpName = "rename_column" @@ -124,6 +125,9 @@ func (v *Operations) UnmarshalJSON(data []byte) error { case OpRawSQLName: item = &OpRawSQL{} + case OpCreateConstraintName: + item = &OpCreateConstraint{} + default: return fmt.Errorf("unknown migration type: %v", opName) } @@ -210,6 +214,9 @@ func OperationName(op Operation) OpName { case *OpRawSQL: return OpRawSQLName + case *OpCreateConstraint: + return OpCreateConstraintName + } panic(fmt.Errorf("unknown operation for %T", op)) From 244d16b03913a398f80af3760b4520b6535dc466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 14:58:54 +0100 Subject: [PATCH 18/64] more fixes --- examples/.ledger | 1 + ...dd_foreign_key_table_reference_constraint.json | 8 +++----- examples/45_add_table_check_constraint.json | 15 +++++++++++++++ pkg/migrations/op_create_constraint.go | 4 ++-- 4 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 examples/45_add_table_check_constraint.json diff --git a/examples/.ledger b/examples/.ledger index f0d13579..3b6b0076 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -42,3 +42,4 @@ 42_create_unique_index.json 43_create_tickets_table.json 44_add_foreign_key_table_reference_constraint.json +45_add_table_check_constraint.json diff --git a/examples/44_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json index 25711657..cb3d8129 100644 --- a/examples/44_add_foreign_key_table_reference_constraint.json +++ b/examples/44_add_foreign_key_table_reference_constraint.json @@ -6,15 +6,13 @@ "type": "foreign_key", "table": "tickets", "name": "fk_sellers", - "columns": ["sellers_name", "sellers_name"], + "columns": ["sellers_name", "sellers_zip"], "references": { "table": "sellers", "name": "fk_sellers", - "column": "name", + "columns": ["name", "zip"], "on_delete": "CASCADE" - }, - "up": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM sellers WHERE sellers.name = sellers_name) THEN sellers_name ELSE NULL END)", - "down": "sellers_name" + } } } ] diff --git a/examples/45_add_table_check_constraint.json b/examples/45_add_table_check_constraint.json new file mode 100644 index 00000000..79705d50 --- /dev/null +++ b/examples/45_add_table_check_constraint.json @@ -0,0 +1,15 @@ +{ + "name": "45_add_table_check_constraint", + "operations": [ + { + "create_constraint": { + "type": "check", + "table": "tickets", + "name": "check_zip_name", + "check": "sellers_name IS NOT NULL AND sellers_zip < 1000", + "up": "(SELECT 'noname', 999)", + "down": "(SELECT sellers_name, sellers_zip FROM tickets WHERE sellers_name IS NOT NULL AND sellers_zip < 1000)" + } + } + ] +} diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 59676612..fdf92514 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -137,7 +137,7 @@ func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB func (o *OpCreateConstraint) addForeignKeyConstraint(ctx context.Context, conn db.DB) error { cols := make([]string, len(o.Columns)) for i, col := range o.Columns { - cols[i] = pq.QuoteIdentifier(TemporaryName(col)) + cols[i] = pq.QuoteIdentifier(col) } onDelete := string(ForeignKeyReferenceOnDeleteNOACTION) @@ -151,7 +151,7 @@ func (o *OpCreateConstraint) addForeignKeyConstraint(ctx context.Context, conn d } else if o.References.Columns != nil { refCols := make([]string, len(o.Columns)) for i, col := range o.References.Columns { - cols[i] = pq.QuoteIdentifier(TemporaryName(col)) + refCols[i] = pq.QuoteIdentifier(col) } references = fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), strings.Join(refCols, ", ")) } From 9fb5c5b521dda40957500a4477d4d5a283b5fce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 14:59:51 +0100 Subject: [PATCH 19/64] rm print --- pkg/migrations/op_add_column.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index da2e6281..c3bde53d 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -210,13 +210,10 @@ func addColumn(ctx context.Context, conn db.DB, o OpAddColumn, t *schema.Table, o.Column.Name = TemporaryName(o.Column.Name) columnWriter := ColumnSQLWriter{WithPK: true, Transformer: tr} colSQL, err := columnWriter.Write(o.Column) - fmt.Println(colSQL) if err != nil { return err } - fmt.Printf("ALTER TABLE %s ADD COLUMN %s\n", pq.QuoteIdentifier(t.Name), colSQL) - _, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s", pq.QuoteIdentifier(t.Name), colSQL, From 3ff9476969af98d58cc33e288bf8c5910385cac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 15:08:13 +0100 Subject: [PATCH 20/64] fix formatting --- examples/43_create_tickets_table.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/43_create_tickets_table.json b/examples/43_create_tickets_table.json index 3a1a48f6..7b144a22 100644 --- a/examples/43_create_tickets_table.json +++ b/examples/43_create_tickets_table.json @@ -22,9 +22,9 @@ "name": "event_id", "type": "integer", "references": { - "name": "fk_events", - "table": "events", - "on_delete": "CASCADE" + "name": "fk_events", + "table": "events", + "on_delete": "CASCADE" } } ] From e6938a22afc6f051fd394a8a662abe53581de3be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 6 Nov 2024 15:11:02 +0100 Subject: [PATCH 21/64] really fix --- examples/43_create_tickets_table.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/43_create_tickets_table.json b/examples/43_create_tickets_table.json index 7b144a22..2f14971b 100644 --- a/examples/43_create_tickets_table.json +++ b/examples/43_create_tickets_table.json @@ -21,10 +21,10 @@ { "name": "event_id", "type": "integer", - "references": { - "name": "fk_events", - "table": "events", - "on_delete": "CASCADE" + "references": { + "name": "fk_events", + "table": "events", + "on_delete": "CASCADE" } } ] From 66995acb426cfc70a2d9375077df99f31f9dc708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 7 Nov 2024 19:25:42 +0100 Subject: [PATCH 22/64] format --- ...d_foreign_key_table_reference_constraint.json | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/examples/44_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json index cb3d8129..63235e47 100644 --- a/examples/44_add_foreign_key_table_reference_constraint.json +++ b/examples/44_add_foreign_key_table_reference_constraint.json @@ -6,12 +6,18 @@ "type": "foreign_key", "table": "tickets", "name": "fk_sellers", - "columns": ["sellers_name", "sellers_zip"], + "columns": [ + "sellers_name", + "sellers_zip" + ], "references": { - "table": "sellers", - "name": "fk_sellers", - "columns": ["name", "zip"], - "on_delete": "CASCADE" + "table": "sellers", + "name": "fk_sellers", + "columns": [ + "name", + "zip" + ], + "on_delete": "CASCADE" } } } From df7e426b59a6f7fa2b0d29d9ba67d6c57c65a108 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 11:36:50 +0100 Subject: [PATCH 23/64] more cleaning --- ...oreign_key_table_reference_constraint.json | 1 - pkg/migrations/fk_reference.go | 22 +++++----- pkg/migrations/op_add_column.go | 4 +- pkg/migrations/op_create_constraint.go | 25 ++++++------ pkg/migrations/op_set_fk.go | 4 +- pkg/migrations/types.go | 20 ++++++++-- schema.json | 40 ++++++++++++++++--- 7 files changed, 79 insertions(+), 37 deletions(-) diff --git a/examples/44_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json index 63235e47..2bcf126e 100644 --- a/examples/44_add_foreign_key_table_reference_constraint.json +++ b/examples/44_add_foreign_key_table_reference_constraint.json @@ -12,7 +12,6 @@ ], "references": { "table": "sellers", - "name": "fk_sellers", "columns": [ "name", "zip" diff --git a/pkg/migrations/fk_reference.go b/pkg/migrations/fk_reference.go index 98f7491b..2af85f6f 100644 --- a/pkg/migrations/fk_reference.go +++ b/pkg/migrations/fk_reference.go @@ -54,16 +54,18 @@ func (f *ForeignKeyReference) Validate(s *schema.Schema) error { } } - switch strings.ToUpper(string(f.OnDelete)) { - case string(ForeignKeyReferenceOnDeleteNOACTION): - case string(ForeignKeyReferenceOnDeleteRESTRICT): - case string(ForeignKeyReferenceOnDeleteSETDEFAULT): - case string(ForeignKeyReferenceOnDeleteSETNULL): - case string(ForeignKeyReferenceOnDeleteCASCADE): - case "": - break - default: - return InvalidOnDeleteSettingError{Name: f.Name, Setting: string(f.OnDelete)} + if f.OnDelete != nil { + switch strings.ToUpper(string(*f.OnDelete)) { + case string(ForeignKeyReferenceOnDeleteNOACTION): + case string(ForeignKeyReferenceOnDeleteRESTRICT): + case string(ForeignKeyReferenceOnDeleteSETDEFAULT): + case string(ForeignKeyReferenceOnDeleteSETNULL): + case string(ForeignKeyReferenceOnDeleteCASCADE): + case "": + break + default: + return InvalidOnDeleteSettingError{Name: f.Name, Setting: string(*f.OnDelete)} + } } return nil diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index c3bde53d..fd11d8e6 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -280,8 +280,8 @@ func (w ColumnSQLWriter) Write(col Column) (string, error) { } if col.References != nil { onDelete := "NO ACTION" - if col.References.OnDelete != "" { - onDelete = strings.ToUpper(string(col.References.OnDelete)) + if col.References.OnDelete != nil { + onDelete = strings.ToUpper(string(*col.References.OnDelete)) } fkName := pq.QuoteIdentifier(col.References.Name) references := pq.QuoteIdentifier(col.References.Table) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index fdf92514..5dc2a009 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -102,7 +102,13 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err if o.References == nil { return FieldRequiredError{Name: "references"} } - if err := o.References.Validate(s); err != nil { + fkReference := ForeignKeyReference{ + Name: o.Name, + Table: o.References.Table, + Columns: o.References.Columns, + OnDelete: o.References.OnDelete, + } + if err := fkReference.Validate(s); err != nil { return err } } @@ -141,20 +147,15 @@ func (o *OpCreateConstraint) addForeignKeyConstraint(ctx context.Context, conn d } onDelete := string(ForeignKeyReferenceOnDeleteNOACTION) - if o.References.OnDelete != "" { - onDelete = strings.ToUpper(string(o.References.OnDelete)) + if o.References.OnDelete != nil { + onDelete = strings.ToUpper(string(*o.References.OnDelete)) } - var references string - if o.References.Column != nil { - references = fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), pq.QuoteIdentifier(*o.References.Column)) - } else if o.References.Columns != nil { - refCols := make([]string, len(o.Columns)) - for i, col := range o.References.Columns { - refCols[i] = pq.QuoteIdentifier(col) - } - references = fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), strings.Join(refCols, ", ")) + refCols := make([]string, len(o.Columns)) + for i, col := range o.References.Columns { + refCols[i] = pq.QuoteIdentifier(col) } + references := fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), strings.Join(refCols, ", ")) _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s ON DELETE %s NOT VALID", diff --git a/pkg/migrations/op_set_fk.go b/pkg/migrations/op_set_fk.go index de29f011..63841a32 100644 --- a/pkg/migrations/op_set_fk.go +++ b/pkg/migrations/op_set_fk.go @@ -86,8 +86,8 @@ func (o *OpSetForeignKey) addForeignKeyConstraint(ctx context.Context, conn db.D tempColumnName := TemporaryName(o.Column) onDelete := "NO ACTION" - if o.References.OnDelete != "" { - onDelete = strings.ToUpper(string(o.References.OnDelete)) + if o.References.OnDelete != nil { + onDelete = strings.ToUpper(string(*o.References.OnDelete)) } var references string diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 600891ac..839e4ff0 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -55,8 +55,20 @@ type ForeignKeyReference struct { // Name of the foreign key constraint Name string `json:"name"` - // On delete behavior of the foreign key constraint - OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` + // Foreign key constraint for the column + OnDelete *ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` + + // Name of the referenced table + Table string `json:"table"` +} + +// Foreign key reference definition +type ForeignKeyReferenceInTable struct { + // List of referenced columns + Columns []string `json:"columns"` + + // Foreign key constraint for the column + OnDelete *ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` // Name of the referenced table Table string `json:"table"` @@ -136,8 +148,8 @@ type OpCreateConstraint struct { // Name of the constraint Name string `json:"name"` - // Foreign key constraint for the column - References *ForeignKeyReference `json:"references,omitempty"` + // Foreign key constraint for the table + References *ForeignKeyReferenceInTable `json:"references,omitempty"` // Name of the table Table string `json:"table"` diff --git a/schema.json b/schema.json index 4d91df5d..c042c208 100644 --- a/schema.json +++ b/schema.json @@ -89,16 +89,44 @@ "type": "string" }, "on_delete": { - "description": "On delete behavior of the foreign key constraint", - "type": "string", - "enum": ["NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET DEFAULT"], - "default": "NO ACTION" + "$ref": "#/$defs/ForeignKeyReferenceOnDelete", + "description": "Foreign key constraint for the column" } }, "required": ["name", "table"], "anyOf": [{ "required": ["column"] }, { "required": ["columns"] }], "type": "object" }, + "ForeignKeyReferenceInTable": { + "additionalProperties": false, + "description": "Foreign key reference definition", + "properties": { + "columns": { + "description": "List of referenced columns", + "items": { + "type": "string" + }, + "type": "array" + }, + "table": { + "description": "Name of the referenced table", + "type": "string" + }, + "on_delete": { + "$ref": "#/$defs/ForeignKeyReferenceOnDelete", + "description": "Foreign key constraint for the column" + } + }, + "required": ["table", "columns"], + "type": "object" + }, + "ForeignKeyReferenceOnDelete": { + "additionalProperties": false, + "description": "On delete behavior of the foreign key constraint", + "type": "string", + "enum": ["NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET DEFAULT"], + "default": "NO ACTION" + }, "OpAddColumn": { "additionalProperties": false, "description": "Add column operation", @@ -469,8 +497,8 @@ "description": "Check constraint for table" }, "references": { - "$ref": "#/$defs/ForeignKeyReference", - "description": "Foreign key constraint for the column" + "$ref": "#/$defs/ForeignKeyReferenceInTable", + "description": "Foreign key constraint for the table" }, "up": { "default": "", From 1d7c900dc3e5c8d17d3ac8982ec8a6633f4508f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 11:44:26 +0100 Subject: [PATCH 24/64] add unique example --- examples/.ledger | 1 + examples/46_add_table_unique_constraint.json | 13 +++++++++++++ pkg/migrations/op_create_constraint.go | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 examples/46_add_table_unique_constraint.json diff --git a/examples/.ledger b/examples/.ledger index 3b6b0076..92e37de9 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -43,3 +43,4 @@ 43_create_tickets_table.json 44_add_foreign_key_table_reference_constraint.json 45_add_table_check_constraint.json +46_add_table_unique_constraint.json diff --git a/examples/46_add_table_unique_constraint.json b/examples/46_add_table_unique_constraint.json new file mode 100644 index 00000000..8b8b2db3 --- /dev/null +++ b/examples/46_add_table_unique_constraint.json @@ -0,0 +1,13 @@ +{ + "name": "46_add_table_unique_constraint", + "operations": [ + { + "create_constraint": { + "type": "unique", + "table": "tickets", + "name": "unique_zip_name", + "columns": ["sellers_name", "sellers_zip"] + } + } + ] +} diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 5dc2a009..49958a35 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -129,7 +129,7 @@ func (o *OpCreateConstraint) addCheckConstraint(ctx context.Context, conn db.DB) func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB) error { cols := make([]string, len(o.Columns)) for i, col := range o.Columns { - cols[i] = pq.QuoteIdentifier(TemporaryName(col)) + cols[i] = pq.QuoteIdentifier(col) } _, err := conn.ExecContext(ctx, fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", pq.QuoteIdentifier(o.Name), From 8fee548da1f9bf43cc94c3f16ea1549d52de09cd Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Wed, 6 Nov 2024 11:10:13 +0000 Subject: [PATCH 25/64] Allow adding a column to a table created in the same migration (#449) Allow the `add_column` operation to add a column to a table that was created by an operation earlier in the same migration. The following migration would previously have failed to start: ```json { "name": "43_multiple_ops", "operations": [ { "create_table": { "name": "players", "columns": [ { "name": "id", "type": "serial", "pk": true }, { "name": "name", "type": "varchar(255)", "check": { "name": "name_length_check", "constraint": "length(name) > 2" } } ] } }, { "add_column": { "table": "players", "column": { "name": "rating", "type": "integer", "comment": "hello world", "check": { "name": "rating_check", "constraint": "rating > 0 AND rating < 100" }, "nullable": false } } } ] } ``` As of this PR, the migration can be started. The above migration does not validate yet, but it can be started successfully with the `--skip-validation` flag to the `start` command. Part of #239 --- pkg/migrations/op_add_column.go | 10 ++-- pkg/migrations/op_add_column_test.go | 68 ++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index fd11d8e6..5f9269c7 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -24,19 +24,19 @@ func (o *OpAddColumn) Start(ctx context.Context, conn db.DB, latestSchema string } if o.Column.Comment != nil { - if err := addCommentToColumn(ctx, conn, o.Table, TemporaryName(o.Column.Name), o.Column.Comment); err != nil { + if err := addCommentToColumn(ctx, conn, table.Name, TemporaryName(o.Column.Name), o.Column.Comment); err != nil { return nil, fmt.Errorf("failed to add comment to column: %w", err) } } if !o.Column.IsNullable() && o.Column.Default == nil { - if err := addNotNullConstraint(ctx, conn, o.Table, o.Column.Name, TemporaryName(o.Column.Name)); err != nil { + if err := addNotNullConstraint(ctx, conn, table.Name, o.Column.Name, TemporaryName(o.Column.Name)); err != nil { return nil, fmt.Errorf("failed to add not null constraint: %w", err) } } if o.Column.Check != nil { - if err := o.addCheckConstraint(ctx, conn); err != nil { + if err := o.addCheckConstraint(ctx, table.Name, conn); err != nil { return nil, fmt.Errorf("failed to add check constraint: %w", err) } } @@ -231,9 +231,9 @@ func addNotNullConstraint(ctx context.Context, conn db.DB, table, column, physic return err } -func (o *OpAddColumn) addCheckConstraint(ctx context.Context, conn db.DB) error { +func (o *OpAddColumn) addCheckConstraint(ctx context.Context, tableName string, conn db.DB) error { _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID", - pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(tableName), pq.QuoteIdentifier(o.Column.Check.Name), rewriteCheckExpression(o.Column.Check.Constraint, o.Column.Name), )) diff --git a/pkg/migrations/op_add_column_test.go b/pkg/migrations/op_add_column_test.go index 756a2926..b4f1de8f 100644 --- a/pkg/migrations/op_add_column_test.go +++ b/pkg/migrations/op_add_column_test.go @@ -1494,6 +1494,74 @@ func TestAddColumnDefaultTransformation(t *testing.T) { }, roll.WithSQLTransformer(sqlTransformer)) } +func TestAddColumnToATableCreatedInTheSameMigration(t *testing.T) { + t.Parallel() + + ExecuteTests(t, TestCases{ + { + name: "add column to newly created table", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + }, + }, + }, + &migrations.OpAddColumn{ + Table: "users", + Column: migrations.Column{ + Name: "age", + Type: "integer", + Nullable: ptr(false), + Check: &migrations.CheckConstraint{ + Name: "age_check", + Constraint: "age >= 18", + }, + Comment: ptr("the age of the user"), + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Inserting into the new column on the new table works. + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "Alice", "age": "30", + }) + + // Inserting a value that doesn't meet the check constraint fails. + MustNotInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "Bob", "age": "8", + }, testutils.CheckViolationErrorCode) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Inserting into the new column on the new table works. + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "Bob", "age": "31", + }) + + // Inserting a value that doesn't meet the check constraint fails. + MustNotInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "Carl", "age": "8", + }, testutils.CheckViolationErrorCode) + }, + }, + }, roll.WithSkipValidation(true)) // TODO: remove once this migration can be validated +} + func TestAddColumnInvalidNameLength(t *testing.T) { t.Parallel() From 8297c87fabd522865747c5fcd94d78f7201283c3 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Wed, 6 Nov 2024 12:36:46 +0000 Subject: [PATCH 26/64] Allow adding an index to a table created in the same migration (#451) Allow the `create_index` operation to add an index to a table that was created by an operation earlier in the same migration. The following migration would previously have failed to start: ```json { "name": "43_multiple_ops", "operations": [ { "create_table": { "name": "players", "columns": [ { "name": "id", "type": "serial", "pk": true }, { "name": "name", "type": "varchar(255)", "check": { "name": "name_length_check", "constraint": "length(name) > 2" } } ] } }, { "create_index": { "name": "idx_player_name", "table": "players", "columns": [ "name" ] } } ] } ``` As of this PR the migration can be started. The above migration does not validate yet, but it can be started successfully with the --skip-validation flag to the start command. Part of https://github.com/xataio/pgroll/issues/239 --- pkg/migrations/op_create_index.go | 4 ++- pkg/migrations/op_create_index_test.go | 50 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/pkg/migrations/op_create_index.go b/pkg/migrations/op_create_index.go index bcc2741e..3add5a9c 100644 --- a/pkg/migrations/op_create_index.go +++ b/pkg/migrations/op_create_index.go @@ -16,6 +16,8 @@ import ( var _ Operation = (*OpCreateIndex)(nil) func (o *OpCreateIndex) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) { + table := s.GetTable(o.Table) + // create index concurrently stmtFmt := "CREATE INDEX CONCURRENTLY %s ON %s" if o.Unique != nil && *o.Unique { @@ -23,7 +25,7 @@ func (o *OpCreateIndex) Start(ctx context.Context, conn db.DB, latestSchema stri } stmt := fmt.Sprintf(stmtFmt, pq.QuoteIdentifier(o.Name), - pq.QuoteIdentifier(o.Table)) + pq.QuoteIdentifier(table.Name)) if o.Method != nil { stmt += fmt.Sprintf(" USING %s", string(*o.Method)) diff --git a/pkg/migrations/op_create_index_test.go b/pkg/migrations/op_create_index_test.go index 65932bb1..28ef5342 100644 --- a/pkg/migrations/op_create_index_test.go +++ b/pkg/migrations/op_create_index_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/xataio/pgroll/pkg/migrations" + "github.com/xataio/pgroll/pkg/roll" ) func TestCreateIndex(t *testing.T) { @@ -313,3 +314,52 @@ func TestCreateIndexOnMultipleColumns(t *testing.T) { }, }}) } + +func TestCreateIndexOnTableCreatedInSameMigration(t *testing.T) { + t.Parallel() + + ExecuteTests(t, TestCases{ + { + name: "create index on newly created table", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(false), + }, + }, + }, + &migrations.OpCreateIndex{ + Name: "idx_users_name", + Table: "users", + Columns: []string{"name"}, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The index has been created on the underlying table. + IndexMustExist(t, db, schema, migrations.TemporaryName("users"), "idx_users_name") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The index has been dropped from the the underlying table. + IndexMustNotExist(t, db, schema, "users", "idx_users_name") + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The index remains on the underlying table. + IndexMustExist(t, db, schema, "users", "idx_users_name") + }, + }, + }, roll.WithSkipValidation(true)) // TODO: Remove once this migration passes validation +} From 5419656ab23c2b5613a4ccbd5f9c9015c9224ee6 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 7 Nov 2024 07:12:42 +0000 Subject: [PATCH 27/64] Add a `healthcheck` to the `db` service in `docker-compose.yml` (#452) Add a healthcheck for the `db` service so that scripts can wait for Postgres to be ready to accept connections, eg: ```sh #! /bin/sh docker compose up -d --wait # ... other commands now that Postgres is ready ``` --- docker-compose.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 5767bfe4..db406077 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,3 +9,8 @@ services: - '5432:5432' volumes: - /var/lib/postgresql/data + healthcheck: + test: "pg_isready -h localhost" + interval: 200ms + timeout: 5s + retries: 10 From f4d323b3840f0ae8fc13a286d4f77c1eb80190df Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 7 Nov 2024 09:48:29 +0000 Subject: [PATCH 28/64] Allow adding indexes to columns created in the same migration (#454) Allow the `create_index` operation to add an index to a column that was created by an operation earlier in the same migration. The following migration would previously have failed to start: ```json { "name": "43_multiple_ops", "operations": [ { "create_table": { "name": "players", "columns": [ { "name": "id", "type": "serial", "pk": true }, { "name": "name", "type": "varchar(255)", "check": { "name": "name_length_check", "constraint": "length(name) > 2" } } ] } }, { "add_column": { "table": "players", "column": { "name": "rating", "type": "integer", "comment": "hello world", "check": { "name": "rating_check", "constraint": "rating > 0 AND rating < 100" }, "nullable": false } } }, { "create_index": { "name": "idx_player_rating", "table": "players", "columns": [ "rating" ] } } ] } ``` As of this PR the migration can be started. The above migration does not validate yet, but it can be started successfully with the `--skip-validation` flag to the `start` command. Part of https://github.com/xataio/pgroll/issues/239 --- pkg/migrations/op_create_index.go | 4 +- pkg/migrations/op_create_index_test.go | 60 +++++++++++++++++++++++++- pkg/schema/schema.go | 10 +++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/pkg/migrations/op_create_index.go b/pkg/migrations/op_create_index.go index 3add5a9c..7408471b 100644 --- a/pkg/migrations/op_create_index.go +++ b/pkg/migrations/op_create_index.go @@ -31,7 +31,9 @@ func (o *OpCreateIndex) Start(ctx context.Context, conn db.DB, latestSchema stri stmt += fmt.Sprintf(" USING %s", string(*o.Method)) } - stmt += fmt.Sprintf(" (%s)", strings.Join(quoteColumnNames(o.Columns), ", ")) + stmt += fmt.Sprintf(" (%s)", strings.Join( + quoteColumnNames(table.PhysicalColumnNamesFor(o.Columns...)), ", "), + ) if o.StorageParameters != nil { stmt += fmt.Sprintf(" WITH (%s)", *o.StorageParameters) diff --git a/pkg/migrations/op_create_index_test.go b/pkg/migrations/op_create_index_test.go index 28ef5342..3b9cdc64 100644 --- a/pkg/migrations/op_create_index_test.go +++ b/pkg/migrations/op_create_index_test.go @@ -315,7 +315,7 @@ func TestCreateIndexOnMultipleColumns(t *testing.T) { }}) } -func TestCreateIndexOnTableCreatedInSameMigration(t *testing.T) { +func TestCreateIndexOnObjectsCreatedInSameMigration(t *testing.T) { t.Parallel() ExecuteTests(t, TestCases{ @@ -361,5 +361,61 @@ func TestCreateIndexOnTableCreatedInSameMigration(t *testing.T) { IndexMustExist(t, db, schema, "users", "idx_users_name") }, }, - }, roll.WithSkipValidation(true)) // TODO: Remove once this migration passes validation + { + name: "create index on newly created column", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(false), + }, + }, + }, + }, + }, + { + Name: "02_add_column_and_index", + Operations: migrations.Operations{ + &migrations.OpAddColumn{ + Table: "users", + Column: migrations.Column{ + Name: "age", + Type: "integer", + Nullable: ptr(true), + }, + Up: "18", + }, + &migrations.OpCreateIndex{ + Name: "idx_users_age", + Table: "users", + Columns: []string{"age"}, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The index has been created on the underlying table. + IndexMustExist(t, db, schema, "users", "idx_users_age") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The index has been dropped from the the underlying table. + IndexMustNotExist(t, db, schema, "users", "idx_users_age") + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The index has been created on the underlying table. + IndexMustExist(t, db, schema, "users", "idx_users_age") + }, + }, + }, roll.WithSkipValidation(true)) // TODO: Remove once these migrations pass validation } diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 49f3ab73..1aa9a48d 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -253,6 +253,16 @@ func (t *Table) RenameColumn(from, to string) { delete(t.Columns, from) } +// PhysicalColumnNames returns the physical column names for the given virtual +// column names +func (t *Table) PhysicalColumnNamesFor(columnNames ...string) []string { + physicalNames := make([]string, 0, len(columnNames)) + for _, cn := range columnNames { + physicalNames = append(physicalNames, t.GetColumn(cn).Name) + } + return physicalNames +} + // Make the Schema struct implement the driver.Valuer interface. This method // simply returns the JSON-encoded representation of the struct. func (s Schema) Value() (driver.Value, error) { From 15cafb349f2ffac70117bea2fcd762667e33c63f Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Fri, 8 Nov 2024 09:49:05 +0100 Subject: [PATCH 29/64] Add missing group by (#456) This may get us past the `pgroll.Init` step in older version of Postgres (<= 13) --- pkg/state/init.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/state/init.sql b/pkg/state/init.sql index b97c6ded..d07400fb 100644 --- a/pkg/state/init.sql +++ b/pkg/state/init.sql @@ -170,7 +170,7 @@ BEGIN JOIN pg_am am ON am.oid = cls.relam WHERE indrelid = t.oid::regclass - GROUP BY pi.indexrelid, pi.indisunique, am.amname) as ix_details), + GROUP BY pi.indexrelid, pi.indisunique, pi.indpred, am.amname) as ix_details), 'checkConstraints', (SELECT COALESCE(json_object_agg(cc_details.conname, json_build_object( 'name', cc_details.conname, 'columns', cc_details.columns, From 24339b2d8c3e6947cb5516198fd216c02ff1b0b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 12:01:03 +0100 Subject: [PATCH 30/64] more update --- pkg/migrations/op_add_column_test.go | 2 +- pkg/migrations/op_change_type_test.go | 2 +- pkg/migrations/op_create_table_test.go | 2 +- pkg/migrations/op_set_check_test.go | 2 +- pkg/migrations/op_set_fk_test.go | 83 ++------------------------ pkg/migrations/op_set_notnull_test.go | 2 +- pkg/migrations/op_set_unique_test.go | 2 +- schema.json | 2 +- 8 files changed, 11 insertions(+), 86 deletions(-) diff --git a/pkg/migrations/op_add_column_test.go b/pkg/migrations/op_add_column_test.go index b4f1de8f..2a026c92 100644 --- a/pkg/migrations/op_add_column_test.go +++ b/pkg/migrations/op_add_column_test.go @@ -626,7 +626,7 @@ func TestAddForeignKeyColumn(t *testing.T) { Name: "fk_users_id", Table: "users", Column: ptr("id"), - OnDelete: "CASCADE", + OnDelete: ptr(migrations.ForeignKeyReferenceOnDelete("CASCADE")), }, }, }, diff --git a/pkg/migrations/op_change_type_test.go b/pkg/migrations/op_change_type_test.go index 60e2667e..6bcf47ed 100644 --- a/pkg/migrations/op_change_type_test.go +++ b/pkg/migrations/op_change_type_test.go @@ -194,7 +194,7 @@ func TestChangeColumnType(t *testing.T) { Name: "fk_employee_department", Table: "departments", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), }, }, }, diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 36ddb000..7acfc11b 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -308,7 +308,7 @@ func TestCreateTable(t *testing.T) { Column: ptr("id"), Name: "fk_users_id", Table: "users", - OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), }, }, { diff --git a/pkg/migrations/op_set_check_test.go b/pkg/migrations/op_set_check_test.go index 648148fb..1b9cce20 100644 --- a/pkg/migrations/op_set_check_test.go +++ b/pkg/migrations/op_set_check_test.go @@ -262,7 +262,7 @@ func TestSetCheckConstraint(t *testing.T) { Name: "fk_employee_department", Table: "departments", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), }, }, }, diff --git a/pkg/migrations/op_set_fk_test.go b/pkg/migrations/op_set_fk_test.go index a3433b1d..129d0d15 100644 --- a/pkg/migrations/op_set_fk_test.go +++ b/pkg/migrations/op_set_fk_test.go @@ -485,7 +485,7 @@ func TestSetForeignKey(t *testing.T) { Name: "fk_users_id", Table: "users", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -591,7 +591,7 @@ func TestSetForeignKey(t *testing.T) { Name: "fk_users_id", Table: "users", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteSETNULL), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -701,7 +701,7 @@ func TestSetForeignKey(t *testing.T) { Name: "fk_users_id", Table: "users", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteSETDEFAULT, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteSETDEFAULT), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -906,7 +906,7 @@ func TestSetForeignKey(t *testing.T) { Name: "fk_users_id_1", Table: "users", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1491,80 +1491,5 @@ func TestSetForeignKeyValidation(t *testing.T) { Err: migrations.ColumnDoesNotExistError{Table: "users", Name: "doesntexist"}, }, }, - { - name: "on_delete must be a valid value", - migrations: []migrations.Migration{ - createTablesMigration, - { - Name: "02_add_fk_constraint", - Operations: migrations.Operations{ - &migrations.OpAlterColumn{ - Table: "posts", - Column: "user_id", - References: &migrations.ForeignKeyReference{ - Name: "fk_users_doesntexist", - Table: "users", - Column: ptr("id"), - OnDelete: "invalid", - }, - Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", - Down: "user_id", - }, - }, - }, - }, - wantStartErr: migrations.InvalidOnDeleteSettingError{ - Name: "fk_users_doesntexist", - Setting: "invalid", - }, - }, - { - name: "on_delete can be specified as lowercase", - migrations: []migrations.Migration{ - createTablesMigration, - { - Name: "02_add_fk_constraint", - Operations: migrations.Operations{ - &migrations.OpAlterColumn{ - Table: "posts", - Column: "user_id", - References: &migrations.ForeignKeyReference{ - Name: "fk_users_doesntexist", - Table: "users", - Column: ptr("id"), - OnDelete: "no action", - }, - Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", - Down: "user_id", - }, - }, - }, - }, - wantStartErr: nil, - }, - { - name: "on_delete can be specified as uppercase", - migrations: []migrations.Migration{ - createTablesMigration, - { - Name: "02_add_fk_constraint", - Operations: migrations.Operations{ - &migrations.OpAlterColumn{ - Table: "posts", - Column: "user_id", - References: &migrations.ForeignKeyReference{ - Name: "fk_users_doesntexist", - Table: "users", - Column: ptr("id"), - OnDelete: "SET NULL", - }, - Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", - Down: "user_id", - }, - }, - }, - }, - wantStartErr: nil, - }, }) } diff --git a/pkg/migrations/op_set_notnull_test.go b/pkg/migrations/op_set_notnull_test.go index f27f053b..cf69492c 100644 --- a/pkg/migrations/op_set_notnull_test.go +++ b/pkg/migrations/op_set_notnull_test.go @@ -271,7 +271,7 @@ func TestSetNotNull(t *testing.T) { Name: "fk_employee_department", Table: "departments", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), }, }, }, diff --git a/pkg/migrations/op_set_unique_test.go b/pkg/migrations/op_set_unique_test.go index 20a0ee2e..61b1b405 100644 --- a/pkg/migrations/op_set_unique_test.go +++ b/pkg/migrations/op_set_unique_test.go @@ -308,7 +308,7 @@ func TestSetColumnUnique(t *testing.T) { Name: "fk_employee_department", Table: "departments", Column: ptr("id"), - OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteSETNULL), }, }, }, diff --git a/schema.json b/schema.json index c042c208..a892b3cc 100644 --- a/schema.json +++ b/schema.json @@ -114,7 +114,7 @@ }, "on_delete": { "$ref": "#/$defs/ForeignKeyReferenceOnDelete", - "description": "Foreign key constraint for the column" + "description": "On delete behavior of the foreign key constraint" } }, "required": ["table", "columns"], From ff70be88b840c289fdfdf7600f0334e86412379d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 12:01:13 +0100 Subject: [PATCH 31/64] more --- pkg/migrations/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 839e4ff0..d774808d 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -67,7 +67,7 @@ type ForeignKeyReferenceInTable struct { // List of referenced columns Columns []string `json:"columns"` - // Foreign key constraint for the column + // On delete behavior of the foreign key constraint OnDelete *ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` // Name of the referenced table From 2942d0c3ccf053e3b54001c907bd95593a29cd76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 13:32:33 +0100 Subject: [PATCH 32/64] minor fixes --- examples/46_add_table_unique_constraint.json | 5 ++++- schema.json | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/46_add_table_unique_constraint.json b/examples/46_add_table_unique_constraint.json index 8b8b2db3..73367d5f 100644 --- a/examples/46_add_table_unique_constraint.json +++ b/examples/46_add_table_unique_constraint.json @@ -6,7 +6,10 @@ "type": "unique", "table": "tickets", "name": "unique_zip_name", - "columns": ["sellers_name", "sellers_zip"] + "columns": [ + "sellers_name", + "sellers_zip" + ] } } ] diff --git a/schema.json b/schema.json index a892b3cc..ba1ed84b 100644 --- a/schema.json +++ b/schema.json @@ -94,7 +94,6 @@ } }, "required": ["name", "table"], - "anyOf": [{ "required": ["column"] }, { "required": ["columns"] }], "type": "object" }, "ForeignKeyReferenceInTable": { From 5679e450ad3ad03d6a297bac5af345140c1c6a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 13:40:02 +0100 Subject: [PATCH 33/64] add docs --- docs/README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/README.md b/docs/README.md index 7c75203f..875fd634 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1059,6 +1059,32 @@ Example **drop column** migrations: * [09_drop_column.json](../examples/09_drop_column.json) +### CREATE constraint + +A create constraint operation adds a new constraint to an existing table. + +Only `CHECK`, `FOREIGN KEY`, and `UNIQUE` constraints can be supported. + +**create constraint** operations have this structure: + +```json +{ + "create_constraint": { + "table": "name of table", + "name": "my_unique_constraint", + "columns": ["list of columns"], + "type": "unique" + } +} +``` + +Example **create constraint** migrations: + +* [44_add_foreign_key_table_reference_constraint.json](../examples/44_add_foreign_key_table_reference_constraint.json) +* [45_add_table_check_constraint.json](../examples/45_add_table_check_constraint.json) +* [46_add_table_unique_constraint.json](../examples/46_add_table_unique_constraint.json) + + ### Drop constraint A drop constraint operation drops a constraint from an existing table. From 7c029748da1bf391dfa0db84a54c5112a792f18e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 13:40:26 +0100 Subject: [PATCH 34/64] add more docs --- docs/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/README.md b/docs/README.md index 875fd634..0d3203ac 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1065,6 +1065,8 @@ A create constraint operation adds a new constraint to an existing table. Only `CHECK`, `FOREIGN KEY`, and `UNIQUE` constraints can be supported. +Required fields: `name`, `table`, `type`. + **create constraint** operations have this structure: ```json From 3ad210c2da6b607ba6170c664166e6cdfada1b78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 13:55:11 +0100 Subject: [PATCH 35/64] add test for casing --- pkg/migrations/fk_reference_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/migrations/fk_reference_test.go b/pkg/migrations/fk_reference_test.go index d226e3d3..1aa405ae 100644 --- a/pkg/migrations/fk_reference_test.go +++ b/pkg/migrations/fk_reference_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/xataio/pgroll/pkg/schema" ) func TestForeignKeyReferenceValidate(t *testing.T) { @@ -28,4 +30,17 @@ func TestForeignKeyReferenceValidate(t *testing.T) { err := fk.Validate(nil) assert.EqualError(t, err, `length of "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (64) exceeds maximum length of 63`) }) + t.Run("On delete casing", func(t *testing.T) { + for _, onDelete := range []string{"cascade", "CASCADE", "RESTRICT", "resTRIct", "NO ACtiON", "no action", "SET NULL", "set null", "SET DEFAULT", "set default"} { + t.Run(onDelete, func(t *testing.T) { + fk := &ForeignKeyReference{ + Table: "my_table", + Name: "fk", + OnDelete: ptr(ForeignKeyReferenceOnDelete(onDelete)), + } + err := fk.Validate(&schema.Schema{Tables: map[string]schema.Table{"my_table": {}}}) + assert.NoError(t, err) + }) + } + }) } From ca097be9ddc46a0ef3e7aa9a18f58338543d563c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 8 Nov 2024 15:38:36 +0100 Subject: [PATCH 36/64] pretty tests --- pkg/migrations/fk_reference_test.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/migrations/fk_reference_test.go b/pkg/migrations/fk_reference_test.go index 1aa405ae..5d0f0d5e 100644 --- a/pkg/migrations/fk_reference_test.go +++ b/pkg/migrations/fk_reference_test.go @@ -31,7 +31,18 @@ func TestForeignKeyReferenceValidate(t *testing.T) { assert.EqualError(t, err, `length of "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (64) exceeds maximum length of 63`) }) t.Run("On delete casing", func(t *testing.T) { - for _, onDelete := range []string{"cascade", "CASCADE", "RESTRICT", "resTRIct", "NO ACtiON", "no action", "SET NULL", "set null", "SET DEFAULT", "set default"} { + for _, onDelete := range []string{ + "cascade", + "CASCADE", + "RESTRICT", + "resTRIct", + "NO ACtiON", + "no action", + "SET NULL", + "set null", + "SET DEFAULT", + "set default", + } { t.Run(onDelete, func(t *testing.T) { fk := &ForeignKeyReference{ Table: "my_table", @@ -43,4 +54,13 @@ func TestForeignKeyReferenceValidate(t *testing.T) { }) } }) + t.Run("On delete invalid value", func(t *testing.T) { + fk := &ForeignKeyReference{ + Table: "my_table", + Name: "fk", + OnDelete: ptr(ForeignKeyReferenceOnDelete("no such action")), + } + err := fk.Validate(&schema.Schema{Tables: map[string]schema.Table{"my_table": {}}}) + assert.EqualError(t, err, `foreign key "fk" on_delete setting must be one of: "NO ACTION", "RESTRICT", "SET DEFAULT", "SET NULL" or "CASCADE", not "no such action"`) + }) } From 8706061ca938cfd046dc67b74dedbfb3ff7bc04c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 11 Nov 2024 14:25:19 +0100 Subject: [PATCH 37/64] create_constraint: unique --- docs/README.md | 6 +- examples/.ledger | 4 +- ...oreign_key_table_reference_constraint.json | 24 -- ...on => 44_add_table_unique_constraint.json} | 2 +- examples/45_add_table_check_constraint.json | 15 - pkg/migrations/.#op_add_column_test.go | 1 + pkg/migrations/errors.go | 8 - pkg/migrations/fk_reference.go | 54 +--- pkg/migrations/fk_reference_test.go | 35 --- pkg/migrations/op_add_column.go | 39 +-- pkg/migrations/op_add_column_test.go | 13 +- pkg/migrations/op_alter_column_test.go | 2 +- pkg/migrations/op_change_type_test.go | 4 +- pkg/migrations/op_create_constraint.go | 76 ----- pkg/migrations/op_create_table_test.go | 8 +- pkg/migrations/op_drop_constraint_test.go | 4 +- pkg/migrations/op_drop_not_null_test.go | 2 +- pkg/migrations/op_set_check.go | 9 +- pkg/migrations/op_set_check_test.go | 4 +- pkg/migrations/op_set_fk.go | 16 +- pkg/migrations/op_set_fk_test.go | 283 ++++++------------ pkg/migrations/op_set_notnull_test.go | 4 +- pkg/migrations/op_set_unique_test.go | 4 +- pkg/migrations/types.go | 22 +- schema.json | 46 +-- 25 files changed, 168 insertions(+), 517 deletions(-) delete mode 100644 examples/44_add_foreign_key_table_reference_constraint.json rename examples/{46_add_table_unique_constraint.json => 44_add_table_unique_constraint.json} (84%) delete mode 100644 examples/45_add_table_check_constraint.json create mode 120000 pkg/migrations/.#op_add_column_test.go diff --git a/docs/README.md b/docs/README.md index 0d3203ac..bdd633e2 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1063,7 +1063,7 @@ Example **drop column** migrations: A create constraint operation adds a new constraint to an existing table. -Only `CHECK`, `FOREIGN KEY`, and `UNIQUE` constraints can be supported. +Only `UNIQUE` constraints can be supported. Required fields: `name`, `table`, `type`. @@ -1082,9 +1082,7 @@ Required fields: `name`, `table`, `type`. Example **create constraint** migrations: -* [44_add_foreign_key_table_reference_constraint.json](../examples/44_add_foreign_key_table_reference_constraint.json) -* [45_add_table_check_constraint.json](../examples/45_add_table_check_constraint.json) -* [46_add_table_unique_constraint.json](../examples/46_add_table_unique_constraint.json) +* [44_add_table_unique_constraint.json](../examples/44_add_table_unique_constraint.json) ### Drop constraint diff --git a/examples/.ledger b/examples/.ledger index 92e37de9..a8067c2a 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -41,6 +41,4 @@ 41_add_enum_column.json 42_create_unique_index.json 43_create_tickets_table.json -44_add_foreign_key_table_reference_constraint.json -45_add_table_check_constraint.json -46_add_table_unique_constraint.json +44_add_table_unique_constraint.json diff --git a/examples/44_add_foreign_key_table_reference_constraint.json b/examples/44_add_foreign_key_table_reference_constraint.json deleted file mode 100644 index 2bcf126e..00000000 --- a/examples/44_add_foreign_key_table_reference_constraint.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "name": "44_add_foreign_key_table_reference_constraint", - "operations": [ - { - "create_constraint": { - "type": "foreign_key", - "table": "tickets", - "name": "fk_sellers", - "columns": [ - "sellers_name", - "sellers_zip" - ], - "references": { - "table": "sellers", - "columns": [ - "name", - "zip" - ], - "on_delete": "CASCADE" - } - } - } - ] -} diff --git a/examples/46_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json similarity index 84% rename from examples/46_add_table_unique_constraint.json rename to examples/44_add_table_unique_constraint.json index 73367d5f..40e5ebf7 100644 --- a/examples/46_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -1,5 +1,5 @@ { - "name": "46_add_table_unique_constraint", + "name": "44_add_table_unique_constraint", "operations": [ { "create_constraint": { diff --git a/examples/45_add_table_check_constraint.json b/examples/45_add_table_check_constraint.json deleted file mode 100644 index 79705d50..00000000 --- a/examples/45_add_table_check_constraint.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "name": "45_add_table_check_constraint", - "operations": [ - { - "create_constraint": { - "type": "check", - "table": "tickets", - "name": "check_zip_name", - "check": "sellers_name IS NOT NULL AND sellers_zip < 1000", - "up": "(SELECT 'noname', 999)", - "down": "(SELECT sellers_name, sellers_zip FROM tickets WHERE sellers_name IS NOT NULL AND sellers_zip < 1000)" - } - } - ] -} diff --git a/pkg/migrations/.#op_add_column_test.go b/pkg/migrations/.#op_add_column_test.go new file mode 120000 index 00000000..57247050 --- /dev/null +++ b/pkg/migrations/.#op_add_column_test.go @@ -0,0 +1 @@ +n@huginn.213900:1729868981 \ No newline at end of file diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index 039ad507..a9566fa6 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -113,14 +113,6 @@ func (e ColumnReferenceError) Error() string { e.Err.Error()) } -type PrimaryKeyDoesNotExistError struct { - Table string -} - -func (e PrimaryKeyDoesNotExistError) Error() string { - return fmt.Sprintf("table %q does not have a primary key", e.Table) -} - type CheckConstraintError struct { Table string Column string diff --git a/pkg/migrations/fk_reference.go b/pkg/migrations/fk_reference.go index 2af85f6f..4f891c56 100644 --- a/pkg/migrations/fk_reference.go +++ b/pkg/migrations/fk_reference.go @@ -3,7 +3,6 @@ package migrations import ( - "fmt" "strings" "github.com/xataio/pgroll/pkg/schema" @@ -19,53 +18,26 @@ func (f *ForeignKeyReference) Validate(s *schema.Schema) error { return err } - if f.Columns != nil && f.Column != nil { - return fmt.Errorf("only one of column or columns is allowed") - } - table := s.GetTable(f.Table) if table == nil { return TableDoesNotExistError{Name: f.Table} } - // check if table reference - if f.Columns != nil && len(f.Columns) == 0 { - // check if primary key exists in case of table reference - if len(table.PrimaryKey) == 0 { - return PrimaryKeyDoesNotExistError{Table: f.Table} - } - } - // check if single column reference - if f.Column != nil { - // check if column exists in case of column reference - column := table.GetColumn(*f.Column) - if column == nil { - return ColumnDoesNotExistError{Table: f.Table, Name: *f.Column} - } - } - // check if multiple column reference - if f.Columns != nil { - for _, col := range f.Columns { - // check if column exists in case of column reference - column := table.GetColumn(col) - if column == nil { - return ColumnDoesNotExistError{Table: f.Table, Name: col} - } - } + column := table.GetColumn(f.Column) + if column == nil { + return ColumnDoesNotExistError{Table: f.Table, Name: f.Column} } - if f.OnDelete != nil { - switch strings.ToUpper(string(*f.OnDelete)) { - case string(ForeignKeyReferenceOnDeleteNOACTION): - case string(ForeignKeyReferenceOnDeleteRESTRICT): - case string(ForeignKeyReferenceOnDeleteSETDEFAULT): - case string(ForeignKeyReferenceOnDeleteSETNULL): - case string(ForeignKeyReferenceOnDeleteCASCADE): - case "": - break - default: - return InvalidOnDeleteSettingError{Name: f.Name, Setting: string(*f.OnDelete)} - } + switch strings.ToUpper(string(f.OnDelete)) { + case string(ForeignKeyReferenceOnDeleteNOACTION): + case string(ForeignKeyReferenceOnDeleteRESTRICT): + case string(ForeignKeyReferenceOnDeleteSETDEFAULT): + case string(ForeignKeyReferenceOnDeleteSETNULL): + case string(ForeignKeyReferenceOnDeleteCASCADE): + case "": + break + default: + return InvalidOnDeleteSettingError{Name: f.Name, Setting: string(f.OnDelete)} } return nil diff --git a/pkg/migrations/fk_reference_test.go b/pkg/migrations/fk_reference_test.go index 5d0f0d5e..d226e3d3 100644 --- a/pkg/migrations/fk_reference_test.go +++ b/pkg/migrations/fk_reference_test.go @@ -7,8 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - - "github.com/xataio/pgroll/pkg/schema" ) func TestForeignKeyReferenceValidate(t *testing.T) { @@ -30,37 +28,4 @@ func TestForeignKeyReferenceValidate(t *testing.T) { err := fk.Validate(nil) assert.EqualError(t, err, `length of "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (64) exceeds maximum length of 63`) }) - t.Run("On delete casing", func(t *testing.T) { - for _, onDelete := range []string{ - "cascade", - "CASCADE", - "RESTRICT", - "resTRIct", - "NO ACtiON", - "no action", - "SET NULL", - "set null", - "SET DEFAULT", - "set default", - } { - t.Run(onDelete, func(t *testing.T) { - fk := &ForeignKeyReference{ - Table: "my_table", - Name: "fk", - OnDelete: ptr(ForeignKeyReferenceOnDelete(onDelete)), - } - err := fk.Validate(&schema.Schema{Tables: map[string]schema.Table{"my_table": {}}}) - assert.NoError(t, err) - }) - } - }) - t.Run("On delete invalid value", func(t *testing.T) { - fk := &ForeignKeyReference{ - Table: "my_table", - Name: "fk", - OnDelete: ptr(ForeignKeyReferenceOnDelete("no such action")), - } - err := fk.Validate(&schema.Schema{Tables: map[string]schema.Table{"my_table": {}}}) - assert.EqualError(t, err, `foreign key "fk" on_delete setting must be one of: "NO ACTION", "RESTRICT", "SET DEFAULT", "SET NULL" or "CASCADE", not "no such action"`) - }) } diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index 5f9269c7..b6595683 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -184,6 +184,12 @@ func (o *OpAddColumn) Validate(ctx context.Context, s *schema.Schema) error { return errors.New("adding primary key columns is not supported") } + // Update the schema to ensure that the new column is visible to validation of + // subsequent operations. + table.AddColumn(o.Column.Name, schema.Column{ + Name: TemporaryName(o.Column.Name), + }) + return nil } @@ -235,7 +241,7 @@ func (o *OpAddColumn) addCheckConstraint(ctx context.Context, tableName string, _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID", pq.QuoteIdentifier(tableName), pq.QuoteIdentifier(o.Column.Check.Name), - rewriteCheckExpression(o.Column.Check.Constraint, o.Column.Name), + rewriteCheckExpression(o.Column.Check.Constraint, o.Column.Name, TemporaryName(o.Column.Name)), )) return err } @@ -252,10 +258,10 @@ func IsNotNullConstraintName(name string) bool { // ColumnSQLWriter writes a column to SQL // It can optionally include the primary key constraint +// When creating a table, the primary key constraint is not added to the column definition type ColumnSQLWriter struct { - WithPK bool - WithFKConstraint bool - Transformer SQLTransformer + WithPK bool + Transformer SQLTransformer } func (w ColumnSQLWriter) Write(col Column) (string, error) { @@ -280,28 +286,15 @@ func (w ColumnSQLWriter) Write(col Column) (string, error) { } if col.References != nil { onDelete := "NO ACTION" - if col.References.OnDelete != nil { - onDelete = strings.ToUpper(string(*col.References.OnDelete)) - } - fkName := pq.QuoteIdentifier(col.References.Name) - references := pq.QuoteIdentifier(col.References.Table) - if col.References.Column != nil { - references = fmt.Sprintf("%s(%s)", pq.QuoteIdentifier(col.References.Table), pq.QuoteIdentifier(*col.References.Column)) - } - if col.References.Columns != nil { - quotedCols := make([]string, len(col.References.Columns)) - for i, c := range col.References.Columns { - quotedCols[i] = pq.QuoteIdentifier(c) - } - fkName = "FOREIGN KEY " + pq.QuoteIdentifier(col.References.Name) - references = fmt.Sprintf("%s(%s)", pq.QuoteIdentifier(col.References.Table), strings.Join(quotedCols, ", ")) + if col.References.OnDelete != "" { + onDelete = strings.ToUpper(string(col.References.OnDelete)) } - sql += fmt.Sprintf(" CONSTRAINT %s REFERENCES %s ON DELETE %s", - fkName, - references, + sql += fmt.Sprintf(" CONSTRAINT %s REFERENCES %s(%s) ON DELETE %s", + pq.QuoteIdentifier(col.References.Name), + pq.QuoteIdentifier(col.References.Table), + pq.QuoteIdentifier(col.References.Column), onDelete) - } if col.Check != nil { sql += fmt.Sprintf(" CONSTRAINT %s CHECK (%s)", diff --git a/pkg/migrations/op_add_column_test.go b/pkg/migrations/op_add_column_test.go index 2a026c92..a53e673a 100644 --- a/pkg/migrations/op_add_column_test.go +++ b/pkg/migrations/op_add_column_test.go @@ -309,7 +309,7 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Nullable: ptr(true), }, @@ -411,7 +411,7 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Nullable: ptr(false), }, @@ -514,7 +514,7 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, }, }, @@ -625,8 +625,8 @@ func TestAddForeignKeyColumn(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDelete("CASCADE")), + Column: "id", + OnDelete: "CASCADE", }, }, }, @@ -1524,6 +1524,7 @@ func TestAddColumnToATableCreatedInTheSameMigration(t *testing.T) { Name: "age", Type: "integer", Nullable: ptr(false), + Default: ptr("18"), Check: &migrations.CheckConstraint{ Name: "age_check", Constraint: "age >= 18", @@ -1559,7 +1560,7 @@ func TestAddColumnToATableCreatedInTheSameMigration(t *testing.T) { }, testutils.CheckViolationErrorCode) }, }, - }, roll.WithSkipValidation(true)) // TODO: remove once this migration can be validated + }) } func TestAddColumnInvalidNameLength(t *testing.T) { diff --git a/pkg/migrations/op_alter_column_test.go b/pkg/migrations/op_alter_column_test.go index 6acc1cfc..cf873f24 100644 --- a/pkg/migrations/op_alter_column_test.go +++ b/pkg/migrations/op_alter_column_test.go @@ -416,7 +416,7 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) { Nullable: ptr(false), References: &migrations.ForeignKeyReference{ Table: "events", - Column: ptr("id"), + Column: "id", Name: "person_manages_event_fk", }, }, diff --git a/pkg/migrations/op_change_type_test.go b/pkg/migrations/op_change_type_test.go index 6bcf47ed..90b000fd 100644 --- a/pkg/migrations/op_change_type_test.go +++ b/pkg/migrations/op_change_type_test.go @@ -193,8 +193,8 @@ func TestChangeColumnType(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, }, diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 49958a35..9ca4e714 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -19,12 +19,8 @@ func (o *OpCreateConstraint) Start(ctx context.Context, conn db.DB, latestSchema table := s.GetTable(o.Table) switch o.Type { - case OpCreateConstraintTypeCheck: - return table, o.addCheckConstraint(ctx, conn) case OpCreateConstraintTypeUnique: return table, o.addUniqueConstraint(ctx, conn) - case OpCreateConstraintTypeForeignKey: - return table, o.addForeignKeyConstraint(ctx, conn) } return table, nil @@ -32,28 +28,12 @@ func (o *OpCreateConstraint) Start(ctx context.Context, conn db.DB, latestSchema func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { switch o.Type { - case OpCreateConstraintTypeCheck: - checkOp := &OpSetCheckConstraint{ - Table: o.Table, - Check: CheckConstraint{ - Name: o.Name, - }, - } - return checkOp.Complete(ctx, conn, tr, s) case OpCreateConstraintTypeUnique: uniqueOp := &OpSetUnique{ Table: o.Table, Name: o.Name, } return uniqueOp.Complete(ctx, conn, tr, s) - case OpCreateConstraintTypeForeignKey: - fkOp := &OpSetForeignKey{ - Table: o.Table, - References: ForeignKeyReference{ - Name: o.Name, - }, - } - return fkOp.Complete(ctx, conn, tr, s) } return nil @@ -90,42 +70,15 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err } switch o.Type { - case OpCreateConstraintTypeCheck: - if o.Check == nil || *o.Check == "" { - return FieldRequiredError{Name: "check"} - } case OpCreateConstraintTypeUnique: if len(o.Columns) == 0 { return FieldRequiredError{Name: "columns"} } - case OpCreateConstraintTypeForeignKey: - if o.References == nil { - return FieldRequiredError{Name: "references"} - } - fkReference := ForeignKeyReference{ - Name: o.Name, - Table: o.References.Table, - Columns: o.References.Columns, - OnDelete: o.References.OnDelete, - } - if err := fkReference.Validate(s); err != nil { - return err - } } return nil } -func (o *OpCreateConstraint) addCheckConstraint(ctx context.Context, conn db.DB) error { - _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID", - pq.QuoteIdentifier(o.Table), - pq.QuoteIdentifier(o.Name), - rewriteCheckExpression(*o.Check, o.Columns...), - )) - - return err -} - func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB) error { cols := make([]string, len(o.Columns)) for i, col := range o.Columns { @@ -139,32 +92,3 @@ func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB return err } - -func (o *OpCreateConstraint) addForeignKeyConstraint(ctx context.Context, conn db.DB) error { - cols := make([]string, len(o.Columns)) - for i, col := range o.Columns { - cols[i] = pq.QuoteIdentifier(col) - } - - onDelete := string(ForeignKeyReferenceOnDeleteNOACTION) - if o.References.OnDelete != nil { - onDelete = strings.ToUpper(string(*o.References.OnDelete)) - } - - refCols := make([]string, len(o.Columns)) - for i, col := range o.References.Columns { - refCols[i] = pq.QuoteIdentifier(col) - } - references := fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), strings.Join(refCols, ", ")) - - _, err := conn.ExecContext(ctx, - fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s ON DELETE %s NOT VALID", - pq.QuoteIdentifier(o.Table), - pq.QuoteIdentifier(o.Name), - strings.Join(cols, ", "), - references, - onDelete, - )) - - return err -} diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 7acfc11b..c0e1fae9 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -198,7 +198,7 @@ func TestCreateTable(t *testing.T) { Name: "user_id", Type: "integer", References: &migrations.ForeignKeyReference{ - Column: ptr("id"), + Column: "id", Name: "fk_users_id", Table: "users", }, @@ -305,10 +305,10 @@ func TestCreateTable(t *testing.T) { Name: "user_id", Type: "integer", References: &migrations.ForeignKeyReference{ - Column: ptr("id"), + Column: "id", Name: "fk_users_id", Table: "users", - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), + OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, { @@ -532,7 +532,7 @@ func TestCreateTableValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_doesntexist", Table: "users", - Column: ptr("doesntexist"), + Column: "doesntexist", }, }, { diff --git a/pkg/migrations/op_drop_constraint_test.go b/pkg/migrations/op_drop_constraint_test.go index 38656318..ba8ded37 100644 --- a/pkg/migrations/op_drop_constraint_test.go +++ b/pkg/migrations/op_drop_constraint_test.go @@ -259,7 +259,7 @@ func TestDropConstraint(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -568,7 +568,7 @@ func TestDropConstraint(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: ptr("id"), + Column: "id", }, }, }, diff --git a/pkg/migrations/op_drop_not_null_test.go b/pkg/migrations/op_drop_not_null_test.go index 0b25c211..f82822ee 100644 --- a/pkg/migrations/op_drop_not_null_test.go +++ b/pkg/migrations/op_drop_not_null_test.go @@ -264,7 +264,7 @@ func TestDropNotNull(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: ptr("id"), + Column: "id", }, }, }, diff --git a/pkg/migrations/op_set_check.go b/pkg/migrations/op_set_check.go index 9c6371ba..e3fbe8d3 100644 --- a/pkg/migrations/op_set_check.go +++ b/pkg/migrations/op_set_check.go @@ -86,7 +86,7 @@ func (o *OpSetCheckConstraint) addCheckConstraint(ctx context.Context, conn db.D _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID", pq.QuoteIdentifier(o.Table), pq.QuoteIdentifier(o.Check.Name), - rewriteCheckExpression(o.Check.Constraint, o.Column), + rewriteCheckExpression(o.Check.Constraint, o.Column, TemporaryName(o.Column)), )) return err @@ -97,9 +97,6 @@ func (o *OpSetCheckConstraint) addCheckConstraint(ctx context.Context, conn db.D // On migration start, however, the check is actually applied to the new (temporary) // column. // This function naively rewrites the check expression to apply to the new column. -func rewriteCheckExpression(check string, oldColumn ...string) string { - for _, oldColumn := range oldColumn { - check = strings.ReplaceAll(check, oldColumn, TemporaryName(oldColumn)) - } - return check +func rewriteCheckExpression(check string, oldColumn, newColumn string) string { + return strings.ReplaceAll(check, oldColumn, newColumn) } diff --git a/pkg/migrations/op_set_check_test.go b/pkg/migrations/op_set_check_test.go index 1b9cce20..13552734 100644 --- a/pkg/migrations/op_set_check_test.go +++ b/pkg/migrations/op_set_check_test.go @@ -261,8 +261,8 @@ func TestSetCheckConstraint(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, }, diff --git a/pkg/migrations/op_set_fk.go b/pkg/migrations/op_set_fk.go index 63841a32..87ed048f 100644 --- a/pkg/migrations/op_set_fk.go +++ b/pkg/migrations/op_set_fk.go @@ -86,23 +86,17 @@ func (o *OpSetForeignKey) addForeignKeyConstraint(ctx context.Context, conn db.D tempColumnName := TemporaryName(o.Column) onDelete := "NO ACTION" - if o.References.OnDelete != nil { - onDelete = strings.ToUpper(string(*o.References.OnDelete)) - } - - var references string - if o.References.Column != nil { - references = fmt.Sprintf("%s (%s)", pq.QuoteIdentifier(o.References.Table), pq.QuoteIdentifier(*o.References.Column)) - } else { - references = pq.QuoteIdentifier(o.References.Table) + if o.References.OnDelete != "" { + onDelete = strings.ToUpper(string(o.References.OnDelete)) } _, err := conn.ExecContext(ctx, - fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s ON DELETE %s NOT VALID", + fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s NOT VALID", pq.QuoteIdentifier(o.Table), pq.QuoteIdentifier(o.References.Name), pq.QuoteIdentifier(tempColumnName), - references, + pq.QuoteIdentifier(o.References.Table), + pq.QuoteIdentifier(o.References.Column), onDelete, )) diff --git a/pkg/migrations/op_set_fk_test.go b/pkg/migrations/op_set_fk_test.go index 129d0d15..e110ab5d 100644 --- a/pkg/migrations/op_set_fk_test.go +++ b/pkg/migrations/op_set_fk_test.go @@ -17,7 +17,7 @@ func TestSetForeignKey(t *testing.T) { ExecuteTests(t, TestCases{ { - name: "add foreign key constraint with column reference", + name: "add foreign key constraint", migrations: []migrations.Migration{ { Name: "01_add_tables", @@ -66,171 +66,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), - }, - Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", - Down: "user_id", - }, - }, - }, - }, - afterStart: func(t *testing.T, db *sql.DB, schema string) { - // The new (temporary) `user_id` column should exist on the underlying table. - ColumnMustExist(t, db, schema, "posts", migrations.TemporaryName("user_id")) - - // A temporary FK constraint has been created on the temporary column - NotValidatedForeignKeyMustExist(t, db, schema, "posts", "fk_users_id") - - // Inserting some data into the `users` table works. - MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ - "name": "alice", - }) - MustInsert(t, db, schema, "02_add_fk_constraint", "users", map[string]string{ - "name": "bob", - }) - - // Inserting data into the new `posts` view with a valid user reference works. - MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ - "title": "post by alice", - "user_id": "1", - }) - - // Inserting data into the new `posts` view with an invalid user reference fails. - MustNotInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ - "title": "post by unknown user", - "user_id": "3", - }, testutils.FKViolationErrorCode) - - // The post that was inserted successfully has been backfilled into the old view. - rows := MustSelect(t, db, schema, "01_add_tables", "posts") - assert.Equal(t, []map[string]any{ - {"id": 1, "title": "post by alice", "user_id": 1}, - }, rows) - - // Inserting data into the old `posts` view with a valid user reference works. - MustInsert(t, db, schema, "01_add_tables", "posts", map[string]string{ - "title": "post by bob", - "user_id": "2", - }) - - // Inserting data into the old `posts` view with an invalid user reference also works. - MustInsert(t, db, schema, "01_add_tables", "posts", map[string]string{ - "title": "post by unknown user", - "user_id": "3", - }) - - // The post that was inserted successfully has been backfilled into the new view. - // The post by an unknown user has been backfilled with a NULL user_id. - rows = MustSelect(t, db, schema, "02_add_fk_constraint", "posts") - assert.Equal(t, []map[string]any{ - {"id": 1, "title": "post by alice", "user_id": 1}, - {"id": 3, "title": "post by bob", "user_id": 2}, - {"id": 4, "title": "post by unknown user", "user_id": nil}, - }, rows) - }, - afterRollback: func(t *testing.T, db *sql.DB, schema string) { - // The new (temporary) `user_id` column should not exist on the underlying table. - ColumnMustNotExist(t, db, schema, "posts", migrations.TemporaryName("user_id")) - - // The up function no longer exists. - FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", "user_id")) - // The down function no longer exists. - FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", migrations.TemporaryName("user_id"))) - - // The up trigger no longer exists. - TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", "user_id")) - // The down trigger no longer exists. - TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", migrations.TemporaryName("user_id"))) - }, - afterComplete: func(t *testing.T, db *sql.DB, schema string) { - // The new (temporary) `user_id` column should not exist on the underlying table. - ColumnMustNotExist(t, db, schema, "posts", migrations.TemporaryName("user_id")) - - // A validated foreign key constraint exists on the underlying table. - ValidatedForeignKeyMustExist(t, db, schema, "posts", "fk_users_id") - - // Inserting data into the new `posts` view with a valid user reference works. - MustInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ - "title": "another post by alice", - "user_id": "1", - }) - - // Inserting data into the new `posts` view with an invalid user reference fails. - MustNotInsert(t, db, schema, "02_add_fk_constraint", "posts", map[string]string{ - "title": "post by unknown user", - "user_id": "3", - }, testutils.FKViolationErrorCode) - - // The data in the new `posts` view is as expected. - rows := MustSelect(t, db, schema, "02_add_fk_constraint", "posts") - assert.Equal(t, []map[string]any{ - {"id": 1, "title": "post by alice", "user_id": 1}, - {"id": 3, "title": "post by bob", "user_id": 2}, - {"id": 4, "title": "post by unknown user", "user_id": nil}, - {"id": 5, "title": "another post by alice", "user_id": 1}, - }, rows) - - // The up function no longer exists. - FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", "user_id")) - // The down function no longer exists. - FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName("posts", migrations.TemporaryName("user_id"))) - - // The up trigger no longer exists. - TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", "user_id")) - // The down trigger no longer exists. - TriggerMustNotExist(t, db, schema, "posts", migrations.TriggerName("posts", migrations.TemporaryName("user_id"))) - }, - }, - { - name: "add foreign key constraint with table reference", - migrations: []migrations.Migration{ - { - Name: "01_add_tables", - Operations: migrations.Operations{ - &migrations.OpCreateTable{ - Name: "users", - Columns: []migrations.Column{ - { - Name: "id", - Type: "serial", - Pk: ptr(true), - }, - { - Name: "name", - Type: "text", - }, - }, - }, - &migrations.OpCreateTable{ - Name: "posts", - Columns: []migrations.Column{ - { - Name: "id", - Type: "serial", - Pk: ptr(true), - }, - { - Name: "title", - Type: "text", - }, - { - Name: "user_id", - Type: "integer", - Nullable: ptr(true), - }, - }, - }, - }, - }, - { - Name: "02_add_fk_constraint", - Operations: migrations.Operations{ - &migrations.OpAlterColumn{ - Table: "posts", - Column: "user_id", - References: &migrations.ForeignKeyReference{ - Name: "fk_users_id", - Table: "users", + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -395,7 +231,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -484,8 +320,8 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -590,8 +426,8 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteSETNULL), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -700,8 +536,8 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteSETDEFAULT), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETDEFAULT, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -812,7 +648,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -905,8 +741,8 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id_1", Table: "users", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -922,7 +758,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id_2", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -994,7 +830,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1077,7 +913,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1163,7 +999,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1265,7 +1101,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1334,7 +1170,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1350,7 +1186,7 @@ func TestSetForeignKey(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_id", Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1423,7 +1259,7 @@ func TestSetForeignKeyValidation(t *testing.T) { Column: "user_id", References: &migrations.ForeignKeyReference{ Table: "users", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1450,7 +1286,7 @@ func TestSetForeignKeyValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_doesntexist_id", Table: "doesntexist", - Column: ptr("id"), + Column: "id", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1477,7 +1313,7 @@ func TestSetForeignKeyValidation(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_users_doesntexist", Table: "users", - Column: ptr("doesntexist"), + Column: "doesntexist", }, Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", Down: "user_id", @@ -1491,5 +1327,80 @@ func TestSetForeignKeyValidation(t *testing.T) { Err: migrations.ColumnDoesNotExistError{Table: "users", Name: "doesntexist"}, }, }, + { + name: "on_delete must be a valid value", + migrations: []migrations.Migration{ + createTablesMigration, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_doesntexist", + Table: "users", + Column: "id", + OnDelete: "invalid", + }, + Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", + Down: "user_id", + }, + }, + }, + }, + wantStartErr: migrations.InvalidOnDeleteSettingError{ + Name: "fk_users_doesntexist", + Setting: "invalid", + }, + }, + { + name: "on_delete can be specified as lowercase", + migrations: []migrations.Migration{ + createTablesMigration, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_doesntexist", + Table: "users", + Column: "id", + OnDelete: "no action", + }, + Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", + Down: "user_id", + }, + }, + }, + }, + wantStartErr: nil, + }, + { + name: "on_delete can be specified as uppercase", + migrations: []migrations.Migration{ + createTablesMigration, + { + Name: "02_add_fk_constraint", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "posts", + Column: "user_id", + References: &migrations.ForeignKeyReference{ + Name: "fk_users_doesntexist", + Table: "users", + Column: "id", + OnDelete: "SET NULL", + }, + Up: "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)", + Down: "user_id", + }, + }, + }, + }, + wantStartErr: nil, + }, }) } diff --git a/pkg/migrations/op_set_notnull_test.go b/pkg/migrations/op_set_notnull_test.go index cf69492c..c4bc22e2 100644 --- a/pkg/migrations/op_set_notnull_test.go +++ b/pkg/migrations/op_set_notnull_test.go @@ -270,8 +270,8 @@ func TestSetNotNull(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteCASCADE), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, }, diff --git a/pkg/migrations/op_set_unique_test.go b/pkg/migrations/op_set_unique_test.go index 61b1b405..4dd2055e 100644 --- a/pkg/migrations/op_set_unique_test.go +++ b/pkg/migrations/op_set_unique_test.go @@ -307,8 +307,8 @@ func TestSetColumnUnique(t *testing.T) { References: &migrations.ForeignKeyReference{ Name: "fk_employee_department", Table: "departments", - Column: ptr("id"), - OnDelete: ptr(migrations.ForeignKeyReferenceOnDeleteSETNULL), + Column: "id", + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, }, }, }, diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index d774808d..07070d69 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -56,19 +56,7 @@ type ForeignKeyReference struct { Name string `json:"name"` // Foreign key constraint for the column - OnDelete *ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` - - // Name of the referenced table - Table string `json:"table"` -} - -// Foreign key reference definition -type ForeignKeyReferenceInTable struct { - // List of referenced columns - Columns []string `json:"columns"` - - // On delete behavior of the foreign key constraint - OnDelete *ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` + OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` // Name of the referenced table Table string `json:"table"` @@ -136,9 +124,6 @@ type OpAlterColumn struct { // Add constraint to table operation type OpCreateConstraint struct { - // Check constraint for table - Check *string `json:"check,omitempty"` - // Columns to add constraint to Columns []string `json:"columns,omitempty"` @@ -148,9 +133,6 @@ type OpCreateConstraint struct { // Name of the constraint Name string `json:"name"` - // Foreign key constraint for the table - References *ForeignKeyReferenceInTable `json:"references,omitempty"` - // Name of the table Table string `json:"table"` @@ -163,8 +145,6 @@ type OpCreateConstraint struct { type OpCreateConstraintType string -const OpCreateConstraintTypeCheck OpCreateConstraintType = "check" -const OpCreateConstraintTypeForeignKey OpCreateConstraintType = "foreign_key" const OpCreateConstraintTypeUnique OpCreateConstraintType = "unique" // Create index operation diff --git a/schema.json b/schema.json index ba1ed84b..94366e60 100644 --- a/schema.json +++ b/schema.json @@ -89,43 +89,15 @@ "type": "string" }, "on_delete": { - "$ref": "#/$defs/ForeignKeyReferenceOnDelete", - "description": "Foreign key constraint for the column" + "description": "Foreign key constraint for the column", + "type": "string", + "enum": ["NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET DEFAULT"], + "default": "NO ACTION" } }, "required": ["name", "table"], "type": "object" }, - "ForeignKeyReferenceInTable": { - "additionalProperties": false, - "description": "Foreign key reference definition", - "properties": { - "columns": { - "description": "List of referenced columns", - "items": { - "type": "string" - }, - "type": "array" - }, - "table": { - "description": "Name of the referenced table", - "type": "string" - }, - "on_delete": { - "$ref": "#/$defs/ForeignKeyReferenceOnDelete", - "description": "On delete behavior of the foreign key constraint" - } - }, - "required": ["table", "columns"], - "type": "object" - }, - "ForeignKeyReferenceOnDelete": { - "additionalProperties": false, - "description": "On delete behavior of the foreign key constraint", - "type": "string", - "enum": ["NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET DEFAULT"], - "default": "NO ACTION" - }, "OpAddColumn": { "additionalProperties": false, "description": "Add column operation", @@ -489,15 +461,7 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["check", "unique", "foreign_key"] - }, - "check": { - "type": "string", - "description": "Check constraint for table" - }, - "references": { - "$ref": "#/$defs/ForeignKeyReferenceInTable", - "description": "Foreign key constraint for the table" + "enum": ["unique"] }, "up": { "default": "", From 56e1a5fdbc08dab9b2ce253bbff2a6469c169a5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 11 Nov 2024 14:29:35 +0100 Subject: [PATCH 38/64] rm more --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index bdd633e2..9a8bcedc 100644 --- a/docs/README.md +++ b/docs/README.md @@ -873,7 +873,7 @@ Add foreign key operations add a foreign key constraint to a column. "references": { "name": "name of foreign key reference", "table": "name of referenced table", - "column": "name of referenced column (optional)", + "column": "name of referenced column", "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", }, "up": "SQL expression", From a26d9e614eae2d89165db321a5ceb8e57c93952b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 11 Nov 2024 14:33:03 +0100 Subject: [PATCH 39/64] more changes --- pkg/migrations/.#op_add_column_test.go | 1 - pkg/migrations/duplicate.go | 2 +- pkg/migrations/op_create_index_test.go | 3 +-- 3 files changed, 2 insertions(+), 4 deletions(-) delete mode 120000 pkg/migrations/.#op_add_column_test.go diff --git a/pkg/migrations/.#op_add_column_test.go b/pkg/migrations/.#op_add_column_test.go deleted file mode 120000 index 57247050..00000000 --- a/pkg/migrations/.#op_add_column_test.go +++ /dev/null @@ -1 +0,0 @@ -n@huginn.213900:1729868981 \ No newline at end of file diff --git a/pkg/migrations/duplicate.go b/pkg/migrations/duplicate.go index e02b5c7b..c6ce3d07 100644 --- a/pkg/migrations/duplicate.go +++ b/pkg/migrations/duplicate.go @@ -136,7 +136,7 @@ func (d *Duplicator) Duplicate(ctx context.Context) error { sql := fmt.Sprintf(cAlterTableAddCheckConstraintSQL, pq.QuoteIdentifier(d.table.Name), pq.QuoteIdentifier(DuplicationName(cc.Name)), - rewriteCheckExpression(cc.Definition, d.column.Name), + rewriteCheckExpression(cc.Definition, d.column.Name, d.asName), ) _, err := d.conn.ExecContext(ctx, sql) diff --git a/pkg/migrations/op_create_index_test.go b/pkg/migrations/op_create_index_test.go index 3b9cdc64..62eaeb5f 100644 --- a/pkg/migrations/op_create_index_test.go +++ b/pkg/migrations/op_create_index_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/xataio/pgroll/pkg/migrations" - "github.com/xataio/pgroll/pkg/roll" ) func TestCreateIndex(t *testing.T) { @@ -417,5 +416,5 @@ func TestCreateIndexOnObjectsCreatedInSameMigration(t *testing.T) { IndexMustExist(t, db, schema, "users", "idx_users_age") }, }, - }, roll.WithSkipValidation(true)) // TODO: Remove once these migrations pass validation + }) } From 76ed12f095e9d46365ce5dd4660f0eac49931cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 11 Nov 2024 14:35:30 +0100 Subject: [PATCH 40/64] more more --- pkg/migrations/types.go | 7 ++----- schema.json | 11 ++--------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 07070d69..a69aa39c 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -47,15 +47,12 @@ type Column struct { // Foreign key reference definition type ForeignKeyReference struct { // Name of the referenced column - Column *string `json:"column,omitempty"` - - // List of referenced columns - Columns []string `json:"columns,omitempty"` + Column string `json:"column"` // Name of the foreign key constraint Name string `json:"name"` - // Foreign key constraint for the column + // On delete behavior of the foreign key constraint OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` // Name of the referenced table diff --git a/schema.json b/schema.json index 94366e60..2cdb73ff 100644 --- a/schema.json +++ b/schema.json @@ -73,13 +73,6 @@ "description": "Name of the referenced column", "type": "string" }, - "columns": { - "description": "List of referenced columns", - "items": { - "type": "string" - }, - "type": "array" - }, "name": { "description": "Name of the foreign key constraint", "type": "string" @@ -89,13 +82,13 @@ "type": "string" }, "on_delete": { - "description": "Foreign key constraint for the column", + "description": "On delete behavior of the foreign key constraint", "type": "string", "enum": ["NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET DEFAULT"], "default": "NO ACTION" } }, - "required": ["name", "table"], + "required": ["column", "name", "table"], "type": "object" }, "OpAddColumn": { From d0e99945b608c1b81dbdfe96b4e4569b3de1ed3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 11 Nov 2024 16:22:12 +0100 Subject: [PATCH 41/64] more updates --- examples/43_create_tickets_table.json | 9 -- pkg/migrations/op_create_constraint.go | 21 ++- pkg/migrations/op_create_constraint_test.go | 163 ++++++++++++++++++++ 3 files changed, 178 insertions(+), 15 deletions(-) create mode 100644 pkg/migrations/op_create_constraint_test.go diff --git a/examples/43_create_tickets_table.json b/examples/43_create_tickets_table.json index 2f14971b..be4be181 100644 --- a/examples/43_create_tickets_table.json +++ b/examples/43_create_tickets_table.json @@ -17,15 +17,6 @@ { "name": "sellers_zip", "type": "integer" - }, - { - "name": "event_id", - "type": "integer", - "references": { - "name": "fk_events", - "table": "events", - "on_delete": "CASCADE" - } } ] } diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 9ca4e714..f4676eea 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -40,7 +40,14 @@ func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTra } func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { - return nil + var err error + switch o.Type { + case OpCreateConstraintTypeUnique: + _, err = conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", + pq.QuoteIdentifier(o.Name))) + } + + return err } func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) error { @@ -80,14 +87,16 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err } func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB) error { - cols := make([]string, len(o.Columns)) - for i, col := range o.Columns { - cols[i] = pq.QuoteIdentifier(col) - } + stmt := fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", + pq.QuoteIdentifier(o.Name), + pq.QuoteIdentifier(o.Table), + strings.Join(quoteColumnNames(o.Columns), ", "), + ) + fmt.Println(stmt) _, err := conn.ExecContext(ctx, fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", pq.QuoteIdentifier(o.Name), pq.QuoteIdentifier(o.Table), - strings.Join(cols, ", "), + strings.Join(quoteColumnNames(o.Columns), ", "), )) return err diff --git a/pkg/migrations/op_create_constraint_test.go b/pkg/migrations/op_create_constraint_test.go new file mode 100644 index 00000000..a2115183 --- /dev/null +++ b/pkg/migrations/op_create_constraint_test.go @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: Apache-2.0 + +package migrations_test + +import ( + "database/sql" + "strings" + "testing" + + "github.com/xataio/pgroll/pkg/migrations" +) + +func TestCreateConstraint(t *testing.T) { + t.Parallel() + + invalidName := strings.Repeat("x", 64) + ExecuteTests(t, TestCases{ + { + name: "create unique constraint on single column", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(false), + }, + }, + }, + }, + }, + { + Name: "02_create_constraint", + Operations: migrations.Operations{ + &migrations.OpCreateConstraint{ + Name: "unique_name", + Table: "users", + Type: "unique", + Columns: []string{"name"}, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The index has been created on the underlying table. + IndexMustExist(t, db, schema, "users", "unique_name") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The index has been dropped from the the underlying table. + IndexMustNotExist(t, db, schema, "users", "uniue_name") + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Complete is a no-op. + }, + }, + { + name: "create unique constraint on multiple columns", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(false), + }, + { + Name: "email", + Type: "varchar(255)", + Nullable: ptr(false), + }, + }, + }, + }, + }, + { + Name: "02_create_index", + Operations: migrations.Operations{ + &migrations.OpCreateConstraint{ + Name: "unique_name_email", + Table: "users", + Type: "unique", + Columns: []string{"name", "email"}, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The index has been created on the underlying table. + IndexMustExist(t, db, schema, "users", "unique_name_email") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The index has been dropped from the the underlying table. + IndexMustNotExist(t, db, schema, "users", "unique_name_email") + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Complete is a no-op. + }, + }, + { + name: "invalid constraint name", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(false), + }, + { + Name: "registered_at_year", + Type: "integer", + Nullable: ptr(false), + }, + }, + }, + }, + }, + { + Name: "02_create_index_with_invalid_name", + Operations: migrations.Operations{ + &migrations.OpCreateConstraint{ + Name: invalidName, + Table: "users", + Columns: []string{"registered_at_year"}, + Type: "unique", + }, + }, + }, + }, + wantStartErr: migrations.ValidateIdentifierLength(invalidName), + afterStart: func(t *testing.T, db *sql.DB, schema string) {}, + afterRollback: func(t *testing.T, db *sql.DB, schema string) {}, + afterComplete: func(t *testing.T, db *sql.DB, schema string) {}, + }, + }) +} From 88ef0a63932ff39adea256cbaf73915048c790a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 11 Nov 2024 16:25:27 +0100 Subject: [PATCH 42/64] quiet linter --- pkg/migrations/op_create_constraint.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index f4676eea..eebbff67 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -18,7 +18,7 @@ var _ Operation = (*OpCreateConstraint)(nil) func (o *OpCreateConstraint) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) { table := s.GetTable(o.Table) - switch o.Type { + switch o.Type { //nolint:gocritic // more cases will be added case OpCreateConstraintTypeUnique: return table, o.addUniqueConstraint(ctx, conn) } @@ -27,7 +27,7 @@ func (o *OpCreateConstraint) Start(ctx context.Context, conn db.DB, latestSchema } func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { - switch o.Type { + switch o.Type { //nolint:gocritic // more cases will be added case OpCreateConstraintTypeUnique: uniqueOp := &OpSetUnique{ Table: o.Table, @@ -41,7 +41,7 @@ func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTra func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { var err error - switch o.Type { + switch o.Type { //nolint:gocritic // more cases will be added case OpCreateConstraintTypeUnique: _, err = conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", pq.QuoteIdentifier(o.Name))) @@ -76,7 +76,7 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err } } - switch o.Type { + switch o.Type { //nolint:gocritic // more cases will be added case OpCreateConstraintTypeUnique: if len(o.Columns) == 0 { return FieldRequiredError{Name: "columns"} From 40de5d164a5c076f5a44127a48f4262554ec047d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 11 Nov 2024 16:28:51 +0100 Subject: [PATCH 43/64] rm professional debug --- pkg/migrations/op_create_constraint.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index eebbff67..356fcb3b 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -87,12 +87,6 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err } func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB) error { - stmt := fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", - pq.QuoteIdentifier(o.Name), - pq.QuoteIdentifier(o.Table), - strings.Join(quoteColumnNames(o.Columns), ", "), - ) - fmt.Println(stmt) _, err := conn.ExecContext(ctx, fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", pq.QuoteIdentifier(o.Name), pq.QuoteIdentifier(o.Table), From 52363ad986f059375bf4bfb08d754162e3b9b328 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 10:11:06 +0100 Subject: [PATCH 44/64] Update pkg/migrations/op_create_constraint.go Co-authored-by: Andrew Farries --- pkg/migrations/op_create_constraint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 356fcb3b..a7708558 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -86,7 +86,7 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err return nil } -func (o *OpCreateConstraint) addUniqueConstraint(ctx context.Context, conn db.DB) error { +func (o *OpCreateConstraint) addUniqueIndex(ctx context.Context, conn db.DB) error { _, err := conn.ExecContext(ctx, fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", pq.QuoteIdentifier(o.Name), pq.QuoteIdentifier(o.Table), From eb6d6f900a48cead036240e4b404869599155039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 10:12:08 +0100 Subject: [PATCH 45/64] Update docs/README.md Co-authored-by: Andrew Farries --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index 9a8bcedc..fcfb5cca 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1059,7 +1059,7 @@ Example **drop column** migrations: * [09_drop_column.json](../examples/09_drop_column.json) -### CREATE constraint +### Create constraint A create constraint operation adds a new constraint to an existing table. From f351a8074708529b741531a414740ab3aec5c2ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 11:33:40 +0100 Subject: [PATCH 46/64] add backfilling --- examples/44_add_table_unique_constraint.json | 2 + pkg/migrations/op_create_constraint.go | 130 ++++++++++++++++++- pkg/migrations/types.go | 4 +- schema.json | 2 +- 4 files changed, 129 insertions(+), 9 deletions(-) diff --git a/examples/44_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json index 40e5ebf7..491880b2 100644 --- a/examples/44_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -10,6 +10,8 @@ "sellers_name", "sellers_zip" ] + "up": "SELECT NULL", + "down": "SELECT sellers_name, sellers_zip" } } ] diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index a7708558..b997726c 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -16,16 +16,66 @@ import ( var _ Operation = (*OpCreateConstraint)(nil) func (o *OpCreateConstraint) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) { - table := s.GetTable(o.Table) + var err error + var table *schema.Table + for _, col := range o.Columns { + if table, err = o.duplicateColumnBeforeStart(ctx, conn, latestSchema, tr, col, s); err != nil { + return nil, err + } + } switch o.Type { //nolint:gocritic // more cases will be added case OpCreateConstraintTypeUnique: - return table, o.addUniqueConstraint(ctx, conn) + return table, o.addUniqueIndex(ctx, conn) } return table, nil } +func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, colName string, s *schema.Schema) (*schema.Table, error) { + table := s.GetTable(o.Table) + column := table.GetColumn(colName) + + d := duplicatorForOperations([]Operation{o}, conn, table, column) + if err := d.Duplicate(ctx); err != nil { + return nil, fmt.Errorf("failed to duplicate column for new constraint: %w", err) + } + + physicalColumnName := TemporaryName(colName) + err := createTrigger(ctx, conn, tr, triggerConfig{ + Name: TriggerName(o.Table, colName), + Direction: TriggerDirectionUp, + Columns: table.Columns, + SchemaName: s.Name, + LatestSchema: latestSchema, + TableName: o.Table, + PhysicalColumn: physicalColumnName, + SQL: o.Up, + }) + if err != nil { + return nil, fmt.Errorf("failed to create up trigger: %w", err) + } + + table.AddColumn(colName, schema.Column{ + Name: physicalColumnName, + }) + + err = createTrigger(ctx, conn, tr, triggerConfig{ + Name: TriggerName(o.Table, physicalColumnName), + Direction: TriggerDirectionDown, + Columns: table.Columns, + LatestSchema: latestSchema, + SchemaName: s.Name, + TableName: o.Table, + PhysicalColumn: colName, + SQL: o.Down, + }) + if err != nil { + return nil, fmt.Errorf("failed to create down trigger: %w", err) + } + return table, nil +} + func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { switch o.Type { //nolint:gocritic // more cases will be added case OpCreateConstraintTypeUnique: @@ -33,23 +83,82 @@ func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTra Table: o.Table, Name: o.Name, } - return uniqueOp.Complete(ctx, conn, tr, s) + err := uniqueOp.Complete(ctx, conn, tr, s) + if err != nil { + return err + } + } + + // remove old columns + _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s %s", + pq.QuoteIdentifier(o.Table), + dropMultipleColumns(quoteColumnNames(o.Columns)), + )) + if err != nil { + return err + } + + // rename new columns to old name + table := s.GetTable(o.Table) + for _, col := range o.Columns { + column := table.GetColumn(col) + if err := RenameDuplicatedColumn(ctx, conn, table, column); err != nil { + return err + } + } + + if err := o.removeTriggers(ctx, conn); err != nil { + return err } return nil } func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { - var err error switch o.Type { //nolint:gocritic // more cases will be added case OpCreateConstraintTypeUnique: - _, err = conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", + _, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", pq.QuoteIdentifier(o.Name))) + if err != nil { + return err + } } + _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s %s", + pq.QuoteIdentifier(o.Table), + dropMultipleColumns(quotedTemporaryNames(o.Columns)), + )) + if err != nil { + return err + } + fmt.Println("remove triggers") + + if err := o.removeTriggers(ctx, conn); err != nil { + return err + } + + return nil +} + +func (o *OpCreateConstraint) removeTriggers(ctx context.Context, conn db.DB) error { + dropFuncs := make([]string, len(o.Columns)*2) + for i, j := 0, 0; i < len(o.Columns); i, j = i+1, j+2 { + dropFuncs[j] = pq.QuoteIdentifier(TriggerFunctionName(o.Table, o.Columns[i])) + dropFuncs[j+1] = pq.QuoteIdentifier(TriggerFunctionName(o.Table, TemporaryName(o.Columns[i]))) + } + _, err := conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE", + strings.Join(dropFuncs, ", "), + )) return err } +func dropMultipleColumns(columns []string) string { + for i, col := range columns { + columns[i] = "DROP COLUMN IF EXISTS " + col + } + return strings.Join(columns, ", ") +} + func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) error { table := s.GetTable(o.Table) if table == nil { @@ -87,11 +196,20 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err } func (o *OpCreateConstraint) addUniqueIndex(ctx context.Context, conn db.DB) error { + fmt.Println(o.Columns) _, err := conn.ExecContext(ctx, fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", pq.QuoteIdentifier(o.Name), pq.QuoteIdentifier(o.Table), - strings.Join(quoteColumnNames(o.Columns), ", "), + strings.Join(quotedTemporaryNames(o.Columns), ", "), )) return err } + +func quotedTemporaryNames(columns []string) []string { + names := make([]string, len(columns)) + for i, col := range columns { + names[i] = TemporaryName(col) + } + return names +} diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index a69aa39c..1af75472 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -125,7 +125,7 @@ type OpCreateConstraint struct { Columns []string `json:"columns,omitempty"` // SQL expression for down migration - Down string `json:"down,omitempty"` + Down string `json:"down"` // Name of the constraint Name string `json:"name"` @@ -137,7 +137,7 @@ type OpCreateConstraint struct { Type OpCreateConstraintType `json:"type"` // SQL expression for up migration - Up string `json:"up,omitempty"` + Up string `json:"up"` } type OpCreateConstraintType string diff --git a/schema.json b/schema.json index 2cdb73ff..42198f24 100644 --- a/schema.json +++ b/schema.json @@ -467,7 +467,7 @@ "type": "string" } }, - "required": ["name", "table", "type"], + "required": ["name", "table", "type", "up", "down"], "type": "object" }, "PgRollOperation": { From 8bd20d1766e84fa0c7cbe58c285db3856c9223cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 11:38:08 +0100 Subject: [PATCH 47/64] update migrations --- examples/44_add_table_unique_constraint.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/44_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json index 491880b2..b8259f27 100644 --- a/examples/44_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -10,8 +10,8 @@ "sellers_name", "sellers_zip" ] - "up": "SELECT NULL", - "down": "SELECT sellers_name, sellers_zip" + "up": "NULL", + "down": "SELECT sellers_name, sellers_zip FROM tickets" } } ] From 0598fed506b91463c6ecd69227b393fb7189e6b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 11:47:27 +0100 Subject: [PATCH 48/64] add comma --- examples/44_add_table_unique_constraint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/44_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json index b8259f27..7d523186 100644 --- a/examples/44_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -9,7 +9,7 @@ "columns": [ "sellers_name", "sellers_zip" - ] + ], "up": "NULL", "down": "SELECT sellers_name, sellers_zip FROM tickets" } From 51573fdf878861a2314bd01c078c719db9887d8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 12:23:24 +0100 Subject: [PATCH 49/64] update down --- examples/44_add_table_unique_constraint.json | 22 ++++++++++++-- pkg/migrations/op_create_constraint.go | 20 +++++++++--- pkg/migrations/types.go | 17 ++++++++--- schema.json | 32 ++++++++++++++++---- 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/examples/44_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json index 7d523186..9a817ce8 100644 --- a/examples/44_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -10,8 +10,26 @@ "sellers_name", "sellers_zip" ], - "up": "NULL", - "down": "SELECT sellers_name, sellers_zip FROM tickets" + "up": [ + { + "column": "sellers_name", + "sql": "NULL" + }, + { + "column": "sellers_zip", + "sql": "NULL" + } + ], + "down": [ + { + "column": "sellers_name", + "sql": "sellers_name" + }, + { + "column": "sellers_zip", + "sql": "sellers_zip" + } + ] } } ] diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index b997726c..4631ecff 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -41,6 +41,13 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con return nil, fmt.Errorf("failed to duplicate column for new constraint: %w", err) } + var upMigration string + for _, mig := range o.Up { + if mig.Column == colName { + upMigration = mig.Sql + break + } + } physicalColumnName := TemporaryName(colName) err := createTrigger(ctx, conn, tr, triggerConfig{ Name: TriggerName(o.Table, colName), @@ -50,7 +57,7 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con LatestSchema: latestSchema, TableName: o.Table, PhysicalColumn: physicalColumnName, - SQL: o.Up, + SQL: upMigration, }) if err != nil { return nil, fmt.Errorf("failed to create up trigger: %w", err) @@ -60,6 +67,13 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con Name: physicalColumnName, }) + var downMigration string + for _, mig := range o.Down { + if mig.Column == colName { + downMigration = mig.Sql + break + } + } err = createTrigger(ctx, conn, tr, triggerConfig{ Name: TriggerName(o.Table, physicalColumnName), Direction: TriggerDirectionDown, @@ -68,7 +82,7 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con SchemaName: s.Name, TableName: o.Table, PhysicalColumn: colName, - SQL: o.Down, + SQL: downMigration, }) if err != nil { return nil, fmt.Errorf("failed to create down trigger: %w", err) @@ -131,7 +145,6 @@ func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTra if err != nil { return err } - fmt.Println("remove triggers") if err := o.removeTriggers(ctx, conn); err != nil { return err @@ -196,7 +209,6 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err } func (o *OpCreateConstraint) addUniqueIndex(ctx context.Context, conn db.DB) error { - fmt.Println(o.Columns) _, err := conn.ExecContext(ctx, fmt.Sprintf("CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS %s ON %s (%s)", pq.QuoteIdentifier(o.Name), pq.QuoteIdentifier(o.Table), diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 1af75472..763a6f30 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -5,6 +5,15 @@ package migrations import "github.com/oapi-codegen/nullable" +// Backfill operation of a column +type BackfillSQL struct { + // Name of the column to backfill + Column string `json:"column"` + + // SQL expression to backfill column + Sql string `json:"sql"` +} + // Check constraint definition type CheckConstraint struct { // Constraint expression @@ -124,8 +133,8 @@ type OpCreateConstraint struct { // Columns to add constraint to Columns []string `json:"columns,omitempty"` - // SQL expression for down migration - Down string `json:"down"` + // SQL expression of down migration for each column + Down []BackfillSQL `json:"down"` // Name of the constraint Name string `json:"name"` @@ -136,8 +145,8 @@ type OpCreateConstraint struct { // Type of the constraint Type OpCreateConstraintType `json:"type"` - // SQL expression for up migration - Up string `json:"up"` + // SQL expression of up migration for each column + Up []BackfillSQL `json:"up"` } type OpCreateConstraintType string diff --git a/schema.json b/schema.json index 42198f24..e260568d 100644 --- a/schema.json +++ b/schema.json @@ -91,6 +91,22 @@ "required": ["column", "name", "table"], "type": "object" }, + "BackfillSQL": { + "description": "Backfill operation of a column", + "additionalProperties": false, + "properties": { + "column": { + "description": "Name of the column to backfill", + "type": "string" + }, + "sql": { + "description": "SQL expression to backfill column", + "type": "string" + } + }, + "required": ["column", "sql"], + "type": "object" + }, "OpAddColumn": { "additionalProperties": false, "description": "Add column operation", @@ -457,14 +473,18 @@ "enum": ["unique"] }, "up": { - "default": "", - "description": "SQL expression for up migration", - "type": "string" + "type": "array", + "items": { + "$ref": "#/$defs/BackfillSQL" + }, + "description": "SQL expression of up migration for each column" }, "down": { - "default": "", - "description": "SQL expression for down migration", - "type": "string" + "type": "array", + "items": { + "$ref": "#/$defs/BackfillSQL" + }, + "description": "SQL expression of down migration for each column" } }, "required": ["name", "table", "type", "up", "down"], From fb7d5177e00f3fb5c249e5b273752c0df7264fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 12:24:05 +0100 Subject: [PATCH 50/64] update readme --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index fcfb5cca..7d15ec76 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1063,7 +1063,7 @@ Example **drop column** migrations: A create constraint operation adds a new constraint to an existing table. -Only `UNIQUE` constraints can be supported. +Only `UNIQUE` constraints are supported. Required fields: `name`, `table`, `type`. From 3d38aecd97ae789a8a2d31172a3fca70f73b2d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 13:27:14 +0100 Subject: [PATCH 51/64] update toc --- docs/README.md | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/docs/README.md b/docs/README.md index 7d15ec76..3d3228b9 100644 --- a/docs/README.md +++ b/docs/README.md @@ -27,6 +27,7 @@ * [Add unique constraint](#add-unique-constraint) * [Create index](#create-index) * [Create table](#create-table) + * [Create constraint](#create-constraint) * [Drop column](#drop-column) * [Drop constraint](#drop-constraint) * [Drop index](#drop-index) @@ -687,6 +688,7 @@ See the [examples](../examples) directory for examples of each kind of operation * [Add unique constraint](#add-unique-constraint) * [Create index](#create-index) * [Create table](#create-table) +* [Create constraint](#create-constraint) * [Drop column](#drop-column) * [Drop constraint](#drop-constraint) * [Drop index](#drop-index) @@ -1037,28 +1039,6 @@ Example **create table** migrations: * [25_add_table_with_check_constraint.json](../examples/25_add_table_with_check_constraint.json) * [28_different_defaults.json](../examples/28_different_defaults.json) -### Drop column - -A drop column operation drops a column from an existing table. - -**drop column** operations have this structure: - -```json -{ - "drop_column": { - "table": "name of table", - "column": "name of column to drop", - "down": "SQL expression" - } -} -``` - -The `down` field above is required in order to backfill the previous version of the schema during an active migration. For instance, in our [example](../examples/09_drop_column.json), you can see that if a new row is inserted against the new schema without a `price` column, the old schema `price` column will be set to `0`. - -Example **drop column** migrations: - -* [09_drop_column.json](../examples/09_drop_column.json) - ### Create constraint A create constraint operation adds a new constraint to an existing table. @@ -1085,6 +1065,28 @@ Example **create constraint** migrations: * [44_add_table_unique_constraint.json](../examples/44_add_table_unique_constraint.json) +### Drop column + +A drop column operation drops a column from an existing table. + +**drop column** operations have this structure: + +```json +{ + "drop_column": { + "table": "name of table", + "column": "name of column to drop", + "down": "SQL expression" + } +} +``` + +The `down` field above is required in order to backfill the previous version of the schema during an active migration. For instance, in our [example](../examples/09_drop_column.json), you can see that if a new row is inserted against the new schema without a `price` column, the old schema `price` column will be set to `0`. + +Example **drop column** migrations: + +* [09_drop_column.json](../examples/09_drop_column.json) + ### Drop constraint A drop constraint operation drops a constraint from an existing table. From 58a7f60b6e93585b0fc8375b749ca5bbc7f410f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 14:32:46 +0100 Subject: [PATCH 52/64] Update examples/44_add_table_unique_constraint.json Co-authored-by: Andrew Farries --- examples/44_add_table_unique_constraint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/44_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json index 9a817ce8..37c8c6be 100644 --- a/examples/44_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -13,7 +13,7 @@ "up": [ { "column": "sellers_name", - "sql": "NULL" + "sql": "sellers_name" }, { "column": "sellers_zip", From f50087f4193fc80b7c4f9b17930a2e8e80b996ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 14:32:55 +0100 Subject: [PATCH 53/64] Update examples/44_add_table_unique_constraint.json Co-authored-by: Andrew Farries --- examples/44_add_table_unique_constraint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/44_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json index 37c8c6be..9a1e12dd 100644 --- a/examples/44_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -17,7 +17,7 @@ }, { "column": "sellers_zip", - "sql": "NULL" + "sql": "sellers_zip" } ], "down": [ From 7c67e9c682c2bd1d09af0d440541875fe7b86479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 14:33:18 +0100 Subject: [PATCH 54/64] Update pkg/migrations/op_create_constraint.go Co-authored-by: Andrew Farries --- pkg/migrations/op_create_constraint.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 4631ecff..3e4a8040 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -146,11 +146,7 @@ func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTra return err } - if err := o.removeTriggers(ctx, conn); err != nil { - return err - } - - return nil + return o.removeTriggers(ctx, conn) } func (o *OpCreateConstraint) removeTriggers(ctx context.Context, conn db.DB) error { From 356e3d654fc59e3603525b656f4db1378613e5ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 14:42:01 +0100 Subject: [PATCH 55/64] Update pkg/migrations/op_create_constraint.go Co-authored-by: Andrew Farries --- pkg/migrations/op_create_constraint.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 3e4a8040..9998e612 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -121,11 +121,7 @@ func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTra } } - if err := o.removeTriggers(ctx, conn); err != nil { - return err - } - - return nil + return o.removeTriggers(ctx, conn) } func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { From 14b19d8a46ceb57ec4534e255cfecf3911881623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 14:49:20 +0100 Subject: [PATCH 56/64] address more review notes --- examples/44_add_table_unique_constraint.json | 28 +++++---------- pkg/migrations/op_create_constraint.go | 38 ++++++++++---------- pkg/migrations/types.go | 23 ++++++------ schema.json | 30 +++------------- 4 files changed, 40 insertions(+), 79 deletions(-) diff --git a/examples/44_add_table_unique_constraint.json b/examples/44_add_table_unique_constraint.json index 9a1e12dd..3ffa9433 100644 --- a/examples/44_add_table_unique_constraint.json +++ b/examples/44_add_table_unique_constraint.json @@ -10,26 +10,14 @@ "sellers_name", "sellers_zip" ], - "up": [ - { - "column": "sellers_name", - "sql": "sellers_name" - }, - { - "column": "sellers_zip", - "sql": "sellers_zip" - } - ], - "down": [ - { - "column": "sellers_name", - "sql": "sellers_name" - }, - { - "column": "sellers_zip", - "sql": "sellers_zip" - } - ] + "up": { + "sellers_name": "sellers_name", + "sellers_zip": "sellers_zip" + }, + "down": { + "sellers_name": "sellers_name", + "sellers_zip": "sellers_zip" + } } } ] diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 9998e612..5cfd83ad 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -36,15 +36,19 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con table := s.GetTable(o.Table) column := table.GetColumn(colName) - d := duplicatorForOperations([]Operation{o}, conn, table, column) + d := NewColumnDuplicator(conn, table, column) if err := d.Duplicate(ctx); err != nil { return nil, fmt.Errorf("failed to duplicate column for new constraint: %w", err) } - var upMigration string - for _, mig := range o.Up { - if mig.Column == colName { - upMigration = mig.Sql + var upSQL string + var ok bool + for col, sql := range o.Up { + if col == colName { + upSQL, ok = sql.(string) + if !ok { + return nil, fmt.Errorf("up migration for column %s is not a string", col) + } break } } @@ -57,7 +61,7 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con LatestSchema: latestSchema, TableName: o.Table, PhysicalColumn: physicalColumnName, - SQL: upMigration, + SQL: upSQL, }) if err != nil { return nil, fmt.Errorf("failed to create up trigger: %w", err) @@ -67,10 +71,13 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con Name: physicalColumnName, }) - var downMigration string - for _, mig := range o.Down { - if mig.Column == colName { - downMigration = mig.Sql + var downSQL string + for col, sql := range o.Up { + if col == colName { + downSQL, ok = sql.(string) + if !ok { + return nil, fmt.Errorf("down migration for column %s is not a string", col) + } break } } @@ -82,7 +89,7 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con SchemaName: s.Name, TableName: o.Table, PhysicalColumn: colName, - SQL: downMigration, + SQL: downSQL, }) if err != nil { return nil, fmt.Errorf("failed to create down trigger: %w", err) @@ -125,15 +132,6 @@ func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTra } func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error { - switch o.Type { //nolint:gocritic // more cases will be added - case OpCreateConstraintTypeUnique: - _, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", - pq.QuoteIdentifier(o.Name))) - if err != nil { - return err - } - } - _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s %s", pq.QuoteIdentifier(o.Table), dropMultipleColumns(quotedTemporaryNames(o.Columns)), diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 763a6f30..b47e679a 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -5,15 +5,6 @@ package migrations import "github.com/oapi-codegen/nullable" -// Backfill operation of a column -type BackfillSQL struct { - // Name of the column to backfill - Column string `json:"column"` - - // SQL expression to backfill column - Sql string `json:"sql"` -} - // Check constraint definition type CheckConstraint struct { // Constraint expression @@ -133,8 +124,8 @@ type OpCreateConstraint struct { // Columns to add constraint to Columns []string `json:"columns,omitempty"` - // SQL expression of down migration for each column - Down []BackfillSQL `json:"down"` + // SQL expression of down migration by column + Down OpCreateConstraintDown `json:"down"` // Name of the constraint Name string `json:"name"` @@ -145,14 +136,20 @@ type OpCreateConstraint struct { // Type of the constraint Type OpCreateConstraintType `json:"type"` - // SQL expression of up migration for each column - Up []BackfillSQL `json:"up"` + // SQL expression of up migration by column + Up OpCreateConstraintUp `json:"up"` } +// SQL expression of down migration by column +type OpCreateConstraintDown map[string]interface{} + type OpCreateConstraintType string const OpCreateConstraintTypeUnique OpCreateConstraintType = "unique" +// SQL expression of up migration by column +type OpCreateConstraintUp map[string]interface{} + // Create index operation type OpCreateIndex struct { // Names of columns on which to define the index diff --git a/schema.json b/schema.json index e260568d..9972406d 100644 --- a/schema.json +++ b/schema.json @@ -91,22 +91,6 @@ "required": ["column", "name", "table"], "type": "object" }, - "BackfillSQL": { - "description": "Backfill operation of a column", - "additionalProperties": false, - "properties": { - "column": { - "description": "Name of the column to backfill", - "type": "string" - }, - "sql": { - "description": "SQL expression to backfill column", - "type": "string" - } - }, - "required": ["column", "sql"], - "type": "object" - }, "OpAddColumn": { "additionalProperties": false, "description": "Add column operation", @@ -473,18 +457,12 @@ "enum": ["unique"] }, "up": { - "type": "array", - "items": { - "$ref": "#/$defs/BackfillSQL" - }, - "description": "SQL expression of up migration for each column" + "type": "object", + "description": "SQL expression of up migration by column" }, "down": { - "type": "array", - "items": { - "$ref": "#/$defs/BackfillSQL" - }, - "description": "SQL expression of down migration for each column" + "type": "object", + "description": "SQL expression of down migration by column" } }, "required": ["name", "table", "type", "up", "down"], From b4a832f65aa4e002896fc8760aa537b377e5cc5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 15:49:57 +0100 Subject: [PATCH 57/64] add more tests --- pkg/migrations/op_create_constraint_test.go | 90 ++++++++++++++++++++- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/pkg/migrations/op_create_constraint_test.go b/pkg/migrations/op_create_constraint_test.go index a2115183..0befd2a6 100644 --- a/pkg/migrations/op_create_constraint_test.go +++ b/pkg/migrations/op_create_constraint_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/xataio/pgroll/internal/testutils" "github.com/xataio/pgroll/pkg/migrations" ) @@ -46,6 +47,12 @@ func TestCreateConstraint(t *testing.T) { Table: "users", Type: "unique", Columns: []string{"name"}, + Up: migrations.OpCreateConstraintUp(map[string]interface{}{ + "name": "name || random()", + }), + Down: migrations.OpCreateConstraintDown(map[string]interface{}{ + "name": "name", + }), }, }, }, @@ -53,13 +60,42 @@ func TestCreateConstraint(t *testing.T) { afterStart: func(t *testing.T, db *sql.DB, schema string) { // The index has been created on the underlying table. IndexMustExist(t, db, schema, "users", "unique_name") + + // Inserting values into the old schema that violate uniqueness should succeed. + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "alice", + }) + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "alice", + }) + + // Inserting values into the new schema that violate uniqueness should fail. + MustInsert(t, db, schema, "02_create_constraint", "users", map[string]string{ + "name": "bob", + }) + MustNotInsert(t, db, schema, "02_create_constraint", "users", map[string]string{ + "name": "bob", + }, testutils.UniqueViolationErrorCode) }, afterRollback: func(t *testing.T, db *sql.DB, schema string) { // The index has been dropped from the the underlying table. IndexMustNotExist(t, db, schema, "users", "uniue_name") + + // Functions, triggers and temporary columns are dropped. + tableCleanedUp(t, db, schema, "users", "name") + }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { - // Complete is a no-op. + // Functions, triggers and temporary columns are dropped. + tableCleanedUp(t, db, schema, "users", "name") + + // Inserting values into the new schema that violate uniqueness should fail. + MustInsert(t, db, schema, "02_create_constraint", "users", map[string]string{ + "name": "carol", + }) + MustNotInsert(t, db, schema, "02_create_constraint", "users", map[string]string{ + "name": "carol", + }, testutils.UniqueViolationErrorCode) }, }, { @@ -91,13 +127,21 @@ func TestCreateConstraint(t *testing.T) { }, }, { - Name: "02_create_index", + Name: "02_create_constraint", Operations: migrations.Operations{ &migrations.OpCreateConstraint{ Name: "unique_name_email", Table: "users", Type: "unique", Columns: []string{"name", "email"}, + Up: migrations.OpCreateConstraintUp(map[string]interface{}{ + "name": "name || random()", + "email": "email || random()", + }), + Down: migrations.OpCreateConstraintDown(map[string]interface{}{ + "name": "name", + "email": "email", + }), }, }, }, @@ -105,10 +149,34 @@ func TestCreateConstraint(t *testing.T) { afterStart: func(t *testing.T, db *sql.DB, schema string) { // The index has been created on the underlying table. IndexMustExist(t, db, schema, "users", "unique_name_email") + + // Inserting values into the old schema that violate uniqueness should succeed. + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "alice", + "email": "alice@alice.me", + }) + MustInsert(t, db, schema, "01_add_table", "users", map[string]string{ + "name": "alice", + "email": "alice@alice.me", + }) + + // Inserting values into the new schema that violate uniqueness should fail. + MustInsert(t, db, schema, "02_create_constraint", "users", map[string]string{ + "name": "bob", + "email": "bob@bob.me", + }) + MustNotInsert(t, db, schema, "02_create_constraint", "users", map[string]string{ + "name": "bob", + "email": "bob@bob.me", + }, testutils.UniqueViolationErrorCode) }, afterRollback: func(t *testing.T, db *sql.DB, schema string) { // The index has been dropped from the the underlying table. IndexMustNotExist(t, db, schema, "users", "unique_name_email") + + // Functions, triggers and temporary columns are dropped. + tableCleanedUp(t, db, schema, "users", "name") + tableCleanedUp(t, db, schema, "users", "email") }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { // Complete is a no-op. @@ -143,7 +211,7 @@ func TestCreateConstraint(t *testing.T) { }, }, { - Name: "02_create_index_with_invalid_name", + Name: "02_create_constraint_with_invalid_name", Operations: migrations.Operations{ &migrations.OpCreateConstraint{ Name: invalidName, @@ -161,3 +229,19 @@ func TestCreateConstraint(t *testing.T) { }, }) } + +func tableCleanedUp(t *testing.T, db *sql.DB, schema, table, column string) { + // The new, temporary column should not exist on the underlying table. + ColumnMustNotExist(t, db, schema, table, migrations.TemporaryName(column)) + + // The up function no longer exists. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName(table, column)) + // The down function no longer exists. + FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName(table, migrations.TemporaryName(column))) + + // The up trigger no longer exists. + TriggerMustNotExist(t, db, schema, table, migrations.TriggerName(table, column)) + // The down trigger no longer exists. + TriggerMustNotExist(t, db, schema, table, migrations.TriggerName(table, migrations.TemporaryName(column))) + +} From c6d72d58fe707b7c25709c5daaffb729c5a2439d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 12 Nov 2024 16:17:03 +0100 Subject: [PATCH 58/64] happy linter --- pkg/migrations/op_create_constraint_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/migrations/op_create_constraint_test.go b/pkg/migrations/op_create_constraint_test.go index 0befd2a6..82cf74c3 100644 --- a/pkg/migrations/op_create_constraint_test.go +++ b/pkg/migrations/op_create_constraint_test.go @@ -83,7 +83,6 @@ func TestCreateConstraint(t *testing.T) { // Functions, triggers and temporary columns are dropped. tableCleanedUp(t, db, schema, "users", "name") - }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { // Functions, triggers and temporary columns are dropped. @@ -243,5 +242,4 @@ func tableCleanedUp(t *testing.T, db *sql.DB, schema, table, column string) { TriggerMustNotExist(t, db, schema, table, migrations.TriggerName(table, column)) // The down trigger no longer exists. TriggerMustNotExist(t, db, schema, table, migrations.TriggerName(table, migrations.TemporaryName(column))) - } From f81b7b91b9b8ee0188e8c0050e29e7a0ed304222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 13 Nov 2024 10:21:04 +0100 Subject: [PATCH 59/64] Update schema.json Co-authored-by: Andrew Farries --- schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/schema.json b/schema.json index 9972406d..2a9c5a8b 100644 --- a/schema.json +++ b/schema.json @@ -458,6 +458,7 @@ }, "up": { "type": "object", + "additionalProperties": { "type": "string" }, "description": "SQL expression of up migration by column" }, "down": { From 9f5aa947c028585d211819eb31fc3e1a0247a874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 13 Nov 2024 10:21:15 +0100 Subject: [PATCH 60/64] Update schema.json Co-authored-by: Andrew Farries --- schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/schema.json b/schema.json index 2a9c5a8b..428c11f1 100644 --- a/schema.json +++ b/schema.json @@ -463,6 +463,7 @@ }, "down": { "type": "object", + "additionalProperties": { "type": "string" }, "description": "SQL expression of down migration by column" } }, From 15ead8358ad4ff4d5b69e6271f64de73510fcbde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 13 Nov 2024 10:31:27 +0100 Subject: [PATCH 61/64] more ntoes --- pkg/migrations/op_create_constraint.go | 25 +++++---------------- pkg/migrations/op_create_constraint_test.go | 8 +++---- pkg/migrations/types.go | 4 ++-- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 5cfd83ad..a4f4e9c3 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -41,16 +41,9 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con return nil, fmt.Errorf("failed to duplicate column for new constraint: %w", err) } - var upSQL string - var ok bool - for col, sql := range o.Up { - if col == colName { - upSQL, ok = sql.(string) - if !ok { - return nil, fmt.Errorf("up migration for column %s is not a string", col) - } - break - } + upSQL, ok := o.Up[colName] + if !ok { + return nil, fmt.Errorf("up migration is missing for column %s", colName) } physicalColumnName := TemporaryName(colName) err := createTrigger(ctx, conn, tr, triggerConfig{ @@ -71,15 +64,9 @@ func (o *OpCreateConstraint) duplicateColumnBeforeStart(ctx context.Context, con Name: physicalColumnName, }) - var downSQL string - for col, sql := range o.Up { - if col == colName { - downSQL, ok = sql.(string) - if !ok { - return nil, fmt.Errorf("down migration for column %s is not a string", col) - } - break - } + downSQL, ok := o.Down[colName] + if !ok { + return nil, fmt.Errorf("down migration is missing for column %s", colName) } err = createTrigger(ctx, conn, tr, triggerConfig{ Name: TriggerName(o.Table, physicalColumnName), diff --git a/pkg/migrations/op_create_constraint_test.go b/pkg/migrations/op_create_constraint_test.go index 82cf74c3..6734b2b0 100644 --- a/pkg/migrations/op_create_constraint_test.go +++ b/pkg/migrations/op_create_constraint_test.go @@ -47,10 +47,10 @@ func TestCreateConstraint(t *testing.T) { Table: "users", Type: "unique", Columns: []string{"name"}, - Up: migrations.OpCreateConstraintUp(map[string]interface{}{ + Up: migrations.OpCreateConstraintUp(map[string]string{ "name": "name || random()", }), - Down: migrations.OpCreateConstraintDown(map[string]interface{}{ + Down: migrations.OpCreateConstraintDown(map[string]string{ "name": "name", }), }, @@ -133,11 +133,11 @@ func TestCreateConstraint(t *testing.T) { Table: "users", Type: "unique", Columns: []string{"name", "email"}, - Up: migrations.OpCreateConstraintUp(map[string]interface{}{ + Up: migrations.OpCreateConstraintUp(map[string]string{ "name": "name || random()", "email": "email || random()", }), - Down: migrations.OpCreateConstraintDown(map[string]interface{}{ + Down: migrations.OpCreateConstraintDown(map[string]string{ "name": "name", "email": "email", }), diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index b47e679a..bd11428b 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -141,14 +141,14 @@ type OpCreateConstraint struct { } // SQL expression of down migration by column -type OpCreateConstraintDown map[string]interface{} +type OpCreateConstraintDown map[string]string type OpCreateConstraintType string const OpCreateConstraintTypeUnique OpCreateConstraintType = "unique" // SQL expression of up migration by column -type OpCreateConstraintUp map[string]interface{} +type OpCreateConstraintUp map[string]string // Create index operation type OpCreateIndex struct { From 32be8485cb4242475bd72de069787d8ea8c1d651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 13 Nov 2024 11:21:26 +0100 Subject: [PATCH 62/64] add check and test --- pkg/migrations/errors.go | 9 +++++ pkg/migrations/op_create_constraint.go | 12 ++++++ pkg/migrations/op_create_constraint_test.go | 44 +++++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index a9566fa6..344d9fab 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -54,6 +54,15 @@ func (e ColumnDoesNotExistError) Error() string { return fmt.Sprintf("column %q does not exist on table %q", e.Name, e.Table) } +type ColumnMigrationMissingError struct { + Table string + Name string +} + +func (e ColumnMigrationMissingError) Error() string { + return fmt.Sprintf("migration for column %q in %q is missing", e.Name, e.Table) +} + type ColumnIsNotNullableError struct { Table string Name string diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index a4f4e9c3..61b9844a 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -173,6 +173,18 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err Name: col, } } + if _, ok := o.Up[col]; !ok { + return ColumnMigrationMissingError{ + Table: o.Table, + Name: col, + } + } + if _, ok := o.Down[col]; !ok { + return ColumnMigrationMissingError{ + Table: o.Table, + Name: col, + } + } } switch o.Type { //nolint:gocritic // more cases will be added diff --git a/pkg/migrations/op_create_constraint_test.go b/pkg/migrations/op_create_constraint_test.go index 6734b2b0..8b2ca8e6 100644 --- a/pkg/migrations/op_create_constraint_test.go +++ b/pkg/migrations/op_create_constraint_test.go @@ -226,6 +226,50 @@ func TestCreateConstraint(t *testing.T) { afterRollback: func(t *testing.T, db *sql.DB, schema string) {}, afterComplete: func(t *testing.T, db *sql.DB, schema string) {}, }, + { + name: "missing migration for constraint creation", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "varchar(255)", + Nullable: ptr(false), + }, + }, + }, + }, + }, + { + Name: "02_create_constraint_with_missing_migration", + Operations: migrations.Operations{ + &migrations.OpCreateConstraint{ + Name: "unique_name", + Table: "users", + Columns: []string{"name"}, + Type: "unique", + Up: migrations.OpCreateConstraintUp(map[string]string{}), + Down: migrations.OpCreateConstraintDown(map[string]string{ + "name": "name", + }), + }, + }, + }, + }, + wantStartErr: migrations.ColumnMigrationMissingError{Table: "users", Name: "name"}, + afterStart: func(t *testing.T, db *sql.DB, schema string) {}, + afterRollback: func(t *testing.T, db *sql.DB, schema string) {}, + afterComplete: func(t *testing.T, db *sql.DB, schema string) {}, + }, }) } From 8e81ec0ed58ed0ec177dcfd8482c1e2ec659c3d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 13 Nov 2024 14:07:22 +0100 Subject: [PATCH 63/64] really quote --- pkg/migrations/op_create_constraint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/migrations/op_create_constraint.go b/pkg/migrations/op_create_constraint.go index 61b9844a..ff8613a7 100644 --- a/pkg/migrations/op_create_constraint.go +++ b/pkg/migrations/op_create_constraint.go @@ -210,7 +210,7 @@ func (o *OpCreateConstraint) addUniqueIndex(ctx context.Context, conn db.DB) err func quotedTemporaryNames(columns []string) []string { names := make([]string, len(columns)) for i, col := range columns { - names[i] = TemporaryName(col) + names[i] = pq.QuoteIdentifier(TemporaryName(col)) } return names } From 031d3e879575d33ab4b6f6d2fbe5326cccd2e619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 13 Nov 2024 14:28:46 +0100 Subject: [PATCH 64/64] update docs --- docs/README.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/README.md b/docs/README.md index 3d3228b9..2094d0c3 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1045,7 +1045,7 @@ A create constraint operation adds a new constraint to an existing table. Only `UNIQUE` constraints are supported. -Required fields: `name`, `table`, `type`. +Required fields: `name`, `table`, `type`, `up`, `down`. **create constraint** operations have this structure: @@ -1054,8 +1054,16 @@ Required fields: `name`, `table`, `type`. "create_constraint": { "table": "name of table", "name": "my_unique_constraint", - "columns": ["list of columns"], + "columns": ["col1", "col2"], "type": "unique" + "up": { + "col1": "col1 || random()", + "col2": "col2 || random()" + }, + "down": { + "col1": "col1", + "col2": "col2" + } } } ```