From b10c298ab8298b1d7acbd2b17c36f29872218569 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 15 Aug 2023 08:20:22 +0100 Subject: [PATCH 1/3] Add example migration for setting uniqueness Create a new table and then add a unique constraint on a pair of columns in a separate migration. --- examples/14_add_reviews_table.json | 29 +++++++++++++++++++++++++++++ examples/15_set_columns_unique.json | 15 +++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 examples/14_add_reviews_table.json create mode 100644 examples/15_set_columns_unique.json diff --git a/examples/14_add_reviews_table.json b/examples/14_add_reviews_table.json new file mode 100644 index 00000000..ed0ffbf6 --- /dev/null +++ b/examples/14_add_reviews_table.json @@ -0,0 +1,29 @@ +{ + "name": "14_add_reviews_table", + "operations": [ + { + "create_table": { + "name": "reviews", + "columns": [ + { + "name": "id", + "type": "serial", + "pk": true + }, + { + "name": "username", + "type": "text" + }, + { + "name": "product", + "type": "text" + }, + { + "name": "review", + "type": "text" + } + ] + } + } + ] +} diff --git a/examples/15_set_columns_unique.json b/examples/15_set_columns_unique.json new file mode 100644 index 00000000..719a0177 --- /dev/null +++ b/examples/15_set_columns_unique.json @@ -0,0 +1,15 @@ +{ + "name": "15_set_columns_unique", + "operations": [ + { + "set_unique": { + "name": "reviews_username_product_unique", + "table": "reviews", + "columns": [ + "username", + "product" + ] + } + } + ] +} From 9de1261f3188d65c015e297cd67f1e768a696988 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 15 Aug 2023 08:26:42 +0100 Subject: [PATCH 2/3] Add tests for the set unique operation --- pkg/migrations/op_common_test.go | 7 ++ pkg/migrations/op_set_unique_test.go | 96 ++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 pkg/migrations/op_set_unique_test.go diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index c81fdb92..8b474fe0 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -250,6 +250,13 @@ func ConstraintMustNotExist(t *testing.T, db *sql.DB, schema, table, constraint } } +func ConstraintMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) { + t.Helper() + if !constraintExists(t, db, schema, table, constraint) { + t.Fatalf("Expected constraint %q to exist", constraint) + } +} + func IndexMustExist(t *testing.T, db *sql.DB, schema, table, index string) { t.Helper() if !indexExists(t, db, schema, table, index) { diff --git a/pkg/migrations/op_set_unique_test.go b/pkg/migrations/op_set_unique_test.go new file mode 100644 index 00000000..d2664b99 --- /dev/null +++ b/pkg/migrations/op_set_unique_test.go @@ -0,0 +1,96 @@ +package migrations_test + +import ( + "database/sql" + "testing" + + "pg-roll/pkg/migrations" +) + +func TestSetColumnsUnique(t *testing.T) { + t.Parallel() + + ExecuteTests(t, TestCases{{ + name: "set unique", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "reviews", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + PrimaryKey: true, + }, + { + Name: "username", + Type: "text", + Nullable: false, + }, + { + Name: "product", + Type: "text", + Nullable: false, + }, + { + Name: "review", + Type: "text", + Nullable: false, + }, + }, + }, + }, + }, + { + Name: "02_set_unique", + Operations: migrations.Operations{ + &migrations.OpSetUnique{ + Table: "reviews", + Columns: []string{"username", "product"}, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB) { + // The unique index has been created on the underlying table. + idxName := migrations.IndexName("reviews", []string{"username", "product"}) + IndexMustExist(t, db, "public", "reviews", idxName) + + // Inserting values into the old schema that violate uniqueness should fail. + MustInsert(t, db, "public", "01_add_table", "reviews", map[string]string{ + "username": "alice", "product": "apple", "review": "good", + }) + MustNotInsert(t, db, "public", "01_add_table", "reviews", map[string]string{ + "username": "alice", "product": "apple", "review": "bad", + }) + + // Inserting values into the new schema that violate uniqueness should fail. + MustInsert(t, db, "public", "02_set_unique", "reviews", map[string]string{ + "username": "bob", "product": "orange", "review": "good", + }) + MustNotInsert(t, db, "public", "02_set_unique", "reviews", map[string]string{ + "username": "bob", "product": "orange", "review": "bad", + }) + }, + afterRollback: func(t *testing.T, db *sql.DB) { + // The unique index has been dropped from the the underlying table. + idxName := migrations.IndexName("reviews", []string{"username", "product"}) + IndexMustNotExist(t, db, "public", "reviews", idxName) + }, + afterComplete: func(t *testing.T, db *sql.DB) { + // The unique constraint has been created on the underlying table. + constraintName := migrations.UniqueConstraintName("reviews", []string{"username", "product"}) + ConstraintMustExist(t, db, "public", "reviews", constraintName) + + // Inserting values into the new schema that violate uniqueness should fail. + MustInsert(t, db, "public", "02_set_unique", "reviews", map[string]string{ + "username": "carl", "product": "banana", "review": "good", + }) + MustNotInsert(t, db, "public", "02_set_unique", "reviews", map[string]string{ + "username": "carl", "product": "banana", "review": "bad", + }) + }, + }}) +} From 5d64ac47d65b6a365d0372d210674d2d6e98fcb8 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Fri, 18 Aug 2023 08:31:04 +0100 Subject: [PATCH 3/3] Implement the set unique operation --- pkg/migrations/op_common.go | 7 +++ pkg/migrations/op_set_unique.go | 65 ++++++++++++++++++++++++++++ pkg/migrations/op_set_unique_test.go | 10 ++--- 3 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 pkg/migrations/op_set_unique.go diff --git a/pkg/migrations/op_common.go b/pkg/migrations/op_common.go index 9ca96639..0b54d8fc 100644 --- a/pkg/migrations/op_common.go +++ b/pkg/migrations/op_common.go @@ -19,6 +19,7 @@ const ( OpNameCreateIndex OpName = "create_index" OpNameDropIndex OpName = "drop_index" OpNameRenameColumn OpName = "rename_column" + OpNameSetUnique OpName = "set_unique" ) func TemporaryName(name string) string { @@ -98,6 +99,9 @@ func (v *Operations) UnmarshalJSON(data []byte) error { case OpNameDropIndex: item = &OpDropIndex{} + case OpNameSetUnique: + item = &OpSetUnique{} + default: return fmt.Errorf("unknown migration type: %v", opName) } @@ -154,6 +158,9 @@ func (v Operations) MarshalJSON() ([]byte, error) { case *OpDropIndex: opName = OpNameDropIndex + case *OpSetUnique: + opName = OpNameSetUnique + default: panic(fmt.Errorf("unknown operation for %T", op)) } diff --git a/pkg/migrations/op_set_unique.go b/pkg/migrations/op_set_unique.go new file mode 100644 index 00000000..09b779ea --- /dev/null +++ b/pkg/migrations/op_set_unique.go @@ -0,0 +1,65 @@ +package migrations + +import ( + "context" + "database/sql" + "fmt" + "strings" + + "github.com/lib/pq" + + "pg-roll/pkg/schema" +) + +type OpSetUnique struct { + Name string `json:"name"` + Table string `json:"table"` + Columns []string `json:"columns"` +} + +var _ Operation = (*OpSetUnique)(nil) + +func (o *OpSetUnique) Start(ctx context.Context, conn *sql.DB, schemaName string, stateSchema string, s *schema.Schema) error { + // create unique index concurrently + _, 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), ", "))) + return err +} + +func (o *OpSetUnique) Complete(ctx context.Context, conn *sql.DB) error { + // create a unique constraint using the unique index + _, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s ADD CONSTRAINT %s UNIQUE USING INDEX %s", + pq.QuoteIdentifier(o.Table), + pq.QuoteIdentifier(o.Name), + pq.QuoteIdentifier(o.Name))) + + return err +} + +func (o *OpSetUnique) Rollback(ctx context.Context, conn *sql.DB) error { + // drop the index concurrently + _, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", o.Name)) + + return err +} + +func (o *OpSetUnique) Validate(ctx context.Context, s *schema.Schema) error { + if o.Name == "" { + return NameRequiredError{} + } + + table := s.GetTable(o.Table) + if table == nil { + return TableDoesNotExistError{Name: o.Table} + } + + for _, column := range o.Columns { + if table.GetColumn(column) == nil { + return ColumnDoesNotExistError{Table: o.Table, Name: column} + } + } + + return nil +} diff --git a/pkg/migrations/op_set_unique_test.go b/pkg/migrations/op_set_unique_test.go index d2664b99..ed8e0b5b 100644 --- a/pkg/migrations/op_set_unique_test.go +++ b/pkg/migrations/op_set_unique_test.go @@ -47,6 +47,7 @@ func TestSetColumnsUnique(t *testing.T) { Name: "02_set_unique", Operations: migrations.Operations{ &migrations.OpSetUnique{ + Name: "reviews_username_product_unique", Table: "reviews", Columns: []string{"username", "product"}, }, @@ -55,8 +56,7 @@ func TestSetColumnsUnique(t *testing.T) { }, afterStart: func(t *testing.T, db *sql.DB) { // The unique index has been created on the underlying table. - idxName := migrations.IndexName("reviews", []string{"username", "product"}) - IndexMustExist(t, db, "public", "reviews", idxName) + IndexMustExist(t, db, "public", "reviews", "reviews_username_product_unique") // Inserting values into the old schema that violate uniqueness should fail. MustInsert(t, db, "public", "01_add_table", "reviews", map[string]string{ @@ -76,13 +76,11 @@ func TestSetColumnsUnique(t *testing.T) { }, afterRollback: func(t *testing.T, db *sql.DB) { // The unique index has been dropped from the the underlying table. - idxName := migrations.IndexName("reviews", []string{"username", "product"}) - IndexMustNotExist(t, db, "public", "reviews", idxName) + IndexMustNotExist(t, db, "public", "reviews", "reviews_username_product_unique") }, afterComplete: func(t *testing.T, db *sql.DB) { // The unique constraint has been created on the underlying table. - constraintName := migrations.UniqueConstraintName("reviews", []string{"username", "product"}) - ConstraintMustExist(t, db, "public", "reviews", constraintName) + ConstraintMustExist(t, db, "public", "reviews", "reviews_username_product_unique") // Inserting values into the new schema that violate uniqueness should fail. MustInsert(t, db, "public", "02_set_unique", "reviews", map[string]string{