Add unique column without exclusive lock

[mcadariu]

Hi,

Awesome project! 👍
This PR is an attempt at fixing #644.

Looking forward to your feedback!

[kvch]

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.

[andrew-farries]

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.

[kvch]

Great start! I added a single note. Thank you for your contribution! 🎉

[agedemenli]

Thanks for the contribution!

Just tested it with

./pgroll init
./pgroll start ./examples/01_create_tables.json --complete
./pgroll start ./examples/02_create_another_table.json --complete 
./pgroll start ./examples/03_add_column.json --complete 

after adding "unique": true for all three columns added in 03_add_column.json
Ended up with below output:

\d+ products
                                                                 Table "public.products"
   Column    │          Type          │ Collation │ Nullable │               Default                │ Storage  │ Compression │ Stats target │ Description
─────────────┼────────────────────────┼───────────┼──────────┼──────────────────────────────────────┼──────────┼─────────────┼──────────────┼─────────────
 id          │ integer                │           │ not null │ nextval('products_id_seq'::regclass) │ plain    │             │              │
 name        │ character varying(255) │           │ not null │                                      │ extended │             │              │
 price       │ numeric(10,2)          │           │ not null │                                      │ main     │             │              │
 description │ character varying(255) │           │          │                                      │ extended │             │              │
 stock       │ integer                │           │ not null │ 100                                  │ plain    │             │              │
 category    │ character varying(255) │           │ not null │                                      │ extended │             │              │
Indexes:
    "products_pkey" PRIMARY KEY, btree (id)
    "products__pgroll_new_category_key" UNIQUE CONSTRAINT, btree (category)
    "products__pgroll_new_description_key" UNIQUE CONSTRAINT, btree (description)
    "products__pgroll_new_stock_key" UNIQUE CONSTRAINT, btree (stock)
    "products_category_key" UNIQUE CONSTRAINT, btree (category)
    "products_description_key" UNIQUE CONSTRAINT, btree (description)
    "products_name_key" UNIQUE CONSTRAINT, btree (name)
    "products_stock_key" UNIQUE CONSTRAINT, btree (stock)
Access method: heap

Looks like now we have double unique constraint for each unique column, one with the actual column name, and one with the temporary one.

[andrew-farries]

Thanks for doing this @mcadariu 👍

Left some comments, I think it's pretty close.

[andrew-farries]

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.

[andrew-farries]

Thanks for the contribution!

Just tested it with

./pgroll init
./pgroll start ./examples/01_create_tables.json --complete
./pgroll start ./examples/02_create_another_table.json --complete 
./pgroll start ./examples/03_add_column.json --complete 

...

I think this is a result of #679 (comment) and I'd expect it to be resolved when we make that change.

[mcadariu]

Thanks all! 🙏

To check the latest version I first ran Ahmet's scenario in the comment above. In ./examples/03_add_column.json I've set all columns to unique:

{
  "name": "03_add_column_to_products",
  "operations": [
    {
      "add_column": {
        "table": "products",
        "up": "UPPER(name)",
        "column": {
          "name": "description",
          "type": "varchar(255)",
          "nullable": true,
          "unique": true
        }
      }
    },
    {
      "add_column": {
        "table": "products",
        "column": {
          "name": "stock",
          "type": "int",
          "nullable": false,
          "default": "100",
          "unique": true
        }
      }
    },
    {
      "add_column": {
        "table": "products",
        "up": "name || '-category'",
        "column": {
          "name": "category",
          "type": "varchar(255)",
          "nullable": false,
          "unique": true
        }
      }
    }
  ]
}

The result looks OK to me now:

postgres=# \d products;
                                      Table "public.products"
   Column    |          Type          | Collation | Nullable |               Default                
-------------+------------------------+-----------+----------+--------------------------------------
 id          | integer                |           | not null | nextval('products_id_seq'::regclass)
 name        | character varying(255) |           | not null | 
 price       | numeric(10,2)          |           | not null | 
 description | character varying(255) |           |          | 
 stock       | integer                |           | not null | 100
 category    | character varying(255) |           | not null | 
Indexes:
    "products_pkey" PRIMARY KEY, btree (id)
    "products_category_key" UNIQUE CONSTRAINT, btree (category)
    "products_description_key" UNIQUE CONSTRAINT, btree (description)
    "products_name_key" UNIQUE CONSTRAINT, btree (name)
    "products_stock_key" UNIQUE CONSTRAINT, btree (stock)

The name column was already set to unique in 02_create_another_table.json.

Next I ran the scenario from the issue description (#644). After step 2, I ran this:

postgres=# SELECT * from pg_locks WHERE relation = 'items'::regclass; \watch 0.1 

Then ran step 3.

I saw only ShareUpdateExclusiveLock, RowShareLock and RowExclusiveLock (no AccessExclusiveLock).

[kvch]

Thank you for the contribution and for making pgroll better! 😄

[gulcin]

Hello @mcadariu, thank you for your contribution to pgroll 🎉 I got so happy to see a familiar face in our project! Have a nice week ahead.

[mcadariu]

Hi @gulcin!
My pleasure! Yes, it's a small world 😄
Likewise, have a good one.