Skip to content

Add unique column without exclusive lock #679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 14, 2025
Merged

Add unique column without exclusive lock #679

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ecd798
Adding a unique column in two steps to avoid exclusive locking
mcadariu Feb 12, 2025
d0d79a8
fix comments
mcadariu Feb 12, 2025
2244822
Use the _pgroll prefix for the temporary unique index name as per the…
mcadariu Feb 13, 2025
85e0864
Avoid setting the UNIQUE constraint directly such that it gets set wi…
mcadariu Feb 13, 2025
34baa4c
Remove superfluous test
mcadariu Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions pkg/migrations/op_add_column.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ func (o *OpAddColumn) Start(ctx context.Context, conn db.DB, latestSchema string
}
}

if o.Column.Unique {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addColumn we already add UNIQUE to the generated SQL. You also need to change that function, so pgroll does not add unique constraint directly to the column. Similarly to what we do for check constraints.

Copy link
Collaborator

@andrew-farries andrew-farries Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be specific, what we need is something like this:

diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go
index 89c97753..a4125e27 100644
--- a/pkg/migrations/op_add_column.go
+++ b/pkg/migrations/op_add_column.go
@@ -252,6 +252,14 @@ func addColumn(ctx context.Context, conn db.DB, o OpAddColumn, t *schema.Table,
 	// This is to avoid unnecessary exclusive table locks.
 	o.Column.Check = nil
 
+	// Don't add a column with a UNIQUE constraint directly.
+	// They are handled by:
+	// - adding the column without the UNIQUE modifier
+	// - creating a UNIQUE index concurrently
+	// - adding a UNIQUE constraint USING the index on migration completion
+	// This is to avoid unnecessary exclusive table locks.
+	o.Column.Unique = false
+
 	o.Column.Name = TemporaryName(o.Column.Name)
 	columnWriter := ColumnSQLWriter{WithPK: true, Transformer: tr}
 	colSQL, err := columnWriter.Write(o.Column)

This ensures that the column is added without the UNIQUE modifier.

if err := createUniqueIndexConcurrently(ctx, conn, s.Name, UniqueIndexName(o.Column.Name), table.Name, []string{TemporaryName(o.Column.Name)}); err != nil {
return nil, fmt.Errorf("failed to add unique index: %w", err)
}
}

var tableToBackfill *schema.Table
if o.Up != "" {
err := createTrigger(ctx, conn, tr, triggerConfig{
Expand Down Expand Up @@ -117,6 +123,17 @@ func (o *OpAddColumn) Complete(ctx context.Context, conn db.DB, tr SQLTransforme
}
}

if o.Column.Unique {
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s_%s_key UNIQUE USING INDEX %s",
pq.QuoteIdentifier(o.Table),
o.Table,
o.Column.Name,
UniqueIndexName(o.Column.Name)))
if err != nil {
return err
}
}

// Add the column to the in-memory schema so that Complete steps in subsequent
// operations can see the new column.
table := s.GetTable(o.Table)
Expand Down Expand Up @@ -235,6 +252,14 @@ func addColumn(ctx context.Context, conn db.DB, o OpAddColumn, t *schema.Table,
// This is to avoid unnecessary exclusive table locks.
o.Column.Check = nil

// Don't add a column with a UNIQUE constraint directly.
// They are handled by:
// - adding the column without the UNIQUE modifier
// - creating a UNIQUE index concurrently
// - adding a UNIQUE constraint USING the index on migration completion
// This is to avoid unnecessary exclusive table locks.
o.Column.Unique = false

o.Column.Name = TemporaryName(o.Column.Name)
columnWriter := ColumnSQLWriter{WithPK: true, Transformer: tr}
colSQL, err := columnWriter.Write(o.Column)
Expand Down Expand Up @@ -268,6 +293,11 @@ func (o *OpAddColumn) addCheckConstraint(ctx context.Context, tableName string,
return err
}

// UniqueIndexName returns the name of the unique index for the given column
func UniqueIndexName(columnName string) string {
return "_pgroll_uniq_" + columnName
}

// NotNullConstraintName returns the name of the NOT NULL constraint for the given column
func NotNullConstraintName(columnName string) string {
return "_pgroll_check_not_null_" + columnName
Expand Down