Remove Column field from DropConstraint (#431)

We can look this up from the schema so can remove it from the spec. This is a breaking change since we remove a field from the schema. Part of #105
xataio · Nov 6, 2024 · 999a299 · 999a299
1 parent 0992c8d
commit 999a299
Show file tree

Hide file tree

Showing 31 changed files with 153 additions and 146 deletions.
diff --git a/docs/README.md b/docs/README.md
@@ -1071,7 +1071,6 @@ Only `CHECK`, `FOREIGN KEY`, and `UNIQUE` constraints can be dropped.
 {
   "drop_constraint": {
     "table": "name of table",
-    "column": "name of column on which constraint is defined",
     "name": "name of constraint to drop",
     "up": "SQL expression",
     "down": "SQL expression"
@@ -1081,9 +1080,9 @@ Only `CHECK`, `FOREIGN KEY`, and `UNIQUE` constraints can be dropped.
 
 Example **drop constraint** migrations:
 
-* [27_drop_unique_constraint.json](../examples/27_drop_unique_constraint.json)
 * [23_drop_check_constraint.json](../examples/23_drop_check_constraint.json)
 * [24_drop_foreign_key_constraint.json](../examples/24_drop_foreign_key_constraint.json)
+* [27_drop_unique_constraint.json](../examples/27_drop_unique_constraint.json)
 
 ### Drop index
 

diff --git a/examples/23_drop_check_constraint.json b/examples/23_drop_check_constraint.json
@@ -4,7 +4,6 @@
     {
       "drop_constraint": {
         "table": "posts",
-        "column": "title",
         "name": "title_length",
         "up": "title",
         "down": "(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)"

diff --git a/examples/24_drop_foreign_key_constraint.json b/examples/24_drop_foreign_key_constraint.json
@@ -4,7 +4,6 @@
     {
       "drop_constraint": {
         "table": "posts",
-        "column": "user_id",
         "name": "fk_users_id",
         "up": "user_id",
         "down": "(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)"

diff --git a/examples/27_drop_unique_constraint.json b/examples/27_drop_unique_constraint.json
@@ -4,7 +4,6 @@
     {
       "drop_constraint": {
         "table": "reviews",
-        "column": "review",
         "name": "reviews_review_unique",
         "up": "review",
         "down": "review || '-' || (random()*1000000)::integer"

diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go
@@ -223,3 +223,12 @@ func ValidateIdentifierLength(name string) error {
 	}
 	return nil
 }
+
+type MultiColumnConstraintsNotSupportedError struct {
+	Table      string
+	Constraint string
+}
+
+func (e MultiColumnConstraintsNotSupportedError) Error() string {
+	return fmt.Sprintf("constraint %q on table %q applies to multiple columns", e.Constraint, e.Table)
+}
diff --git a/pkg/migrations/migrations.go b/pkg/migrations/migrations.go
@@ -28,7 +28,7 @@ type Operation interface {
 
 	// Rollback will revert the changes made by Start. It is not possible to
 	// rollback a completed migration.
-	Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error
+	Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error
 
 	// Validate returns a descriptive error if the operation cannot be applied to the given schema.
 	Validate(ctx context.Context, s *schema.Schema) error

diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go
@@ -119,7 +119,7 @@ func (o *OpAddColumn) Complete(ctx context.Context, conn db.DB, tr SQLTransforme
 	return err
 }
 
-func (o *OpAddColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
+func (o *OpAddColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
 	tempName := TemporaryName(o.Column.Name)
 
 	_, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",

diff --git a/pkg/migrations/op_alter_column.go b/pkg/migrations/op_alter_column.go
@@ -151,12 +151,12 @@ func (o *OpAlterColumn) Complete(ctx context.Context, conn db.DB, tr SQLTransfor
 	return nil
 }
 
-func (o *OpAlterColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
+func (o *OpAlterColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
 	ops := o.subOperations()
 
 	// Perform any operation specific rollback steps
 	for _, ops := range ops {
-		if err := ops.Rollback(ctx, conn, tr); err != nil {
+		if err := ops.Rollback(ctx, conn, tr, nil); err != nil {
 			return err
 		}
 	}

diff --git a/pkg/migrations/op_change_type.go b/pkg/migrations/op_change_type.go
@@ -29,7 +29,7 @@ func (o *OpChangeType) Complete(ctx context.Context, conn db.DB, tr SQLTransform
 	return nil
 }
 
-func (o *OpChangeType) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
+func (o *OpChangeType) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
 	return nil
 }
 

diff --git a/pkg/migrations/op_create_index.go b/pkg/migrations/op_create_index.go
@@ -48,7 +48,7 @@ func (o *OpCreateIndex) Complete(ctx context.Context, conn db.DB, tr SQLTransfor
 	return nil
 }
 
-func (o *OpCreateIndex) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
+func (o *OpCreateIndex) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
 	// drop the index concurrently
 	_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s",
 		pq.QuoteIdentifier(o.Name)))

diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go
@@ -70,7 +70,7 @@ func (o *OpCreateTable) Complete(ctx context.Context, conn db.DB, tr SQLTransfor
 	return err
 }
 
-func (o *OpCreateTable) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
+func (o *OpCreateTable) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
 	tempName := TemporaryName(o.Name)
 
 	_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP TABLE IF EXISTS %s",

diff --git a/pkg/migrations/op_drop_column.go b/pkg/migrations/op_drop_column.go
@@ -48,7 +48,7 @@ func (o *OpDropColumn) Complete(ctx context.Context, conn db.DB, tr SQLTransform
 	return err
 }
 
-func (o *OpDropColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
+func (o *OpDropColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
 	_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE",
 		pq.QuoteIdentifier(TriggerFunctionName(o.Table, o.Column))))
 

diff --git a/pkg/migrations/op_drop_constraint.go b/pkg/migrations/op_drop_constraint.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 
 	"github.com/lib/pq"
+
 	"github.com/xataio/pgroll/pkg/db"
 	"github.com/xataio/pgroll/pkg/schema"
 )
@@ -15,7 +16,10 @@ var _ Operation = (*OpDropConstraint)(nil)
 
 func (o *OpDropConstraint) Start(ctx context.Context, conn db.DB, latestSchema string, tr SQLTransformer, s *schema.Schema, cbs ...CallbackFn) (*schema.Table, error) {
 	table := s.GetTable(o.Table)
-	column := table.GetColumn(o.Column)
+
+	// By this point Validate() should have run which ensures the constraint exists and that we only have
+	// one column associated with it.
+	column := table.GetColumn(table.GetConstraintColumns(o.Name)[0])
 
 	// Create a copy of the column on the underlying table.
 	d := NewColumnDuplicator(conn, table, column).WithoutConstraint(o.Name)
@@ -25,14 +29,14 @@ func (o *OpDropConstraint) Start(ctx context.Context, conn db.DB, latestSchema s
 
 	// Add a trigger to copy values from the old column to the new, rewriting values using the `up` SQL.
 	err := createTrigger(ctx, conn, tr, triggerConfig{
-		Name:           TriggerName(o.Table, o.Column),
+		Name:           TriggerName(o.Table, column.Name),
 		Direction:      TriggerDirectionUp,
 		Columns:        table.Columns,
 		SchemaName:     s.Name,
 		LatestSchema:   latestSchema,
 		TableName:      o.Table,
-		PhysicalColumn: TemporaryName(o.Column),
-		SQL:            o.upSQL(),
+		PhysicalColumn: TemporaryName(column.Name),
+		SQL:            o.upSQL(column.Name),
 	})
 	if err != nil {
 		return nil, fmt.Errorf("failed to create up trigger: %w", err)
@@ -41,19 +45,19 @@ func (o *OpDropConstraint) Start(ctx context.Context, conn db.DB, latestSchema s
 	// Add the new column to the internal schema representation. This is done
 	// here, before creation of the down trigger, so that the trigger can declare
 	// a variable for the new column.
-	table.AddColumn(o.Column, schema.Column{
-		Name: TemporaryName(o.Column),
+	table.AddColumn(column.Name, schema.Column{
+		Name: TemporaryName(column.Name),
 	})
 
 	// Add a trigger to copy values from the new column to the old, rewriting values using the `down` SQL.
 	err = createTrigger(ctx, conn, tr, triggerConfig{
-		Name:           TriggerName(o.Table, TemporaryName(o.Column)),
+		Name:           TriggerName(o.Table, TemporaryName(column.Name)),
 		Direction:      TriggerDirectionDown,
 		Columns:        table.Columns,
 		SchemaName:     s.Name,
 		LatestSchema:   latestSchema,
 		TableName:      o.Table,
-		PhysicalColumn: o.Column,
+		PhysicalColumn: column.Name,
 		SQL:            o.Down,
 	})
 	if err != nil {
@@ -63,59 +67,65 @@ func (o *OpDropConstraint) Start(ctx context.Context, conn db.DB, latestSchema s
 }
 
 func (o *OpDropConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
+	// We have already validated that there is single column related to this constraint.
+	table := s.GetTable(o.Table)
+	column := table.GetColumn(table.GetConstraintColumns(o.Name)[0])
+
 	// Remove the up function and trigger
 	_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE",
-		pq.QuoteIdentifier(TriggerFunctionName(o.Table, o.Column))))
+		pq.QuoteIdentifier(TriggerFunctionName(o.Table, column.Name))))
 	if err != nil {
 		return err
 	}
 
 	// Remove the down function and trigger
 	_, err = conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE",
-		pq.QuoteIdentifier(TriggerFunctionName(o.Table, TemporaryName(o.Column)))))
+		pq.QuoteIdentifier(TriggerFunctionName(o.Table, TemporaryName(column.Name)))))
 	if err != nil {
 		return err
 	}
 
 	// Drop the old column
 	_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",
 		pq.QuoteIdentifier(o.Table),
-		pq.QuoteIdentifier(o.Column)))
+		pq.QuoteIdentifier(column.Name)))
 	if err != nil {
 		return err
 	}
 
 	// Rename the new column to the old column name
-	table := s.GetTable(o.Table)
-	column := table.GetColumn(o.Column)
 	if err := RenameDuplicatedColumn(ctx, conn, table, column); err != nil {
 		return err
 	}
 
 	return err
 }
 
-func (o *OpDropConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
+func (o *OpDropConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
+	// We have already validated that there is single column related to this constraint.
+	table := s.GetTable(o.Table)
+	column := table.GetColumn(table.GetConstraintColumns(o.Name)[0])
+
 	// Drop the new column
 	_, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s DROP COLUMN IF EXISTS %s",
 		pq.QuoteIdentifier(o.Table),
-		pq.QuoteIdentifier(TemporaryName(o.Column)),
+		pq.QuoteIdentifier(TemporaryName(column.Name)),
 	))
 	if err != nil {
 		return err
 	}
 
 	// Remove the up function and trigger
 	_, err = conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE",
-		pq.QuoteIdentifier(TriggerFunctionName(o.Table, o.Column)),
+		pq.QuoteIdentifier(TriggerFunctionName(o.Table, column.Name)),
 	))
 	if err != nil {
 		return err
 	}
 
 	// Remove the down function and trigger
 	_, err = conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE",
-		pq.QuoteIdentifier(TriggerFunctionName(o.Table, TemporaryName(o.Column))),
+		pq.QuoteIdentifier(TriggerFunctionName(o.Table, TemporaryName(column.Name))),
 	))
 
 	return err
@@ -127,11 +137,6 @@ func (o *OpDropConstraint) Validate(ctx context.Context, s *schema.Schema) error
 		return TableDoesNotExistError{Name: o.Table}
 	}
 
-	column := table.GetColumn(o.Column)
-	if column == nil {
-		return ColumnDoesNotExistError{Table: o.Table, Name: o.Column}
-	}
-
 	if o.Name == "" {
 		return FieldRequiredError{Name: "name"}
 	}
@@ -140,17 +145,28 @@ func (o *OpDropConstraint) Validate(ctx context.Context, s *schema.Schema) error
 		return ConstraintDoesNotExistError{Table: o.Table, Constraint: o.Name}
 	}
 
+	columns := table.GetConstraintColumns(o.Name)
+
+	// We already know the constraint exists because we checked it earlier so we only need to check the
+	// case where there are multiple columns.
+	if len(columns) > 1 {
+		return MultiColumnConstraintsNotSupportedError{
+			Table:      table.Name,
+			Constraint: o.Name,
+		}
+	}
+
 	if o.Down == "" {
 		return FieldRequiredError{Name: "down"}
 	}
 
 	return nil
 }
 
-func (o *OpDropConstraint) upSQL() string {
+func (o *OpDropConstraint) upSQL(column string) string {
 	if o.Up != "" {
 		return o.Up
 	}
 
-	return pq.QuoteIdentifier(o.Column)
+	return pq.QuoteIdentifier(column)
 }