Added check for add_unique_constraint - closes #252

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.7.0 (unreleased)
+
+- Added check for `add_unique_constraint`
+
 ## 1.6.4 (2023-10-17)
 
 - Fixed false positives with `revert`

README.md

@@ -75,6 +75,7 @@ Postgres-specific checks:
 - [adding an index non-concurrently](#adding-an-index-non-concurrently)
 - [adding a reference](#adding-a-reference)
 - [adding a foreign key](#adding-a-foreign-key)
+- [adding a unique constraint](#adding-a-unique-constraint) [unreleased]
 - [adding an exclusion constraint](#adding-an-exclusion-constraint)
 - [adding a json column](#adding-a-json-column)
 - [setting NOT NULL on an existing column](#setting-not-null-on-an-existing-column)
@@ -512,6 +513,39 @@ class ValidateForeignKeyOnUsers < ActiveRecord::Migration[7.1]
 end
 ```
 
+### Adding a unique constraint
+
+#### Bad
+
+In Postgres, adding a unique constraint creates a unique index, which blocks reads and writes.
+
+```ruby
+class AddUniqueContraint < ActiveRecord::Migration[7.1]
+  def change
+    add_unique_constraint :users, :some_column
+  end
+end
+```
+
+#### Good
+
+Create a unique index concurrently, then use it for the constraint.
+
+```ruby
+class AddUniqueContraint < ActiveRecord::Migration[7.1]
+  disable_ddl_transaction!
+
+  def up
+    add_index :users, :some_column, unique: true, algorithm: :concurrently
+    add_unique_constraint :users, using_index: "index_users_on_some_column"
+  end
+
+  def down
+    remove_unique_constraint :users, :some_column
+  end
+end
+```
+
 ### Adding an exclusion constraint
 
 #### Bad

lib/strong_migrations/checker.rb

@@ -48,6 +48,8 @@ def perform(method, *args)
           check_add_index(*args)
         when :add_reference, :add_belongs_to
           check_add_reference(method, *args)
+        when :add_unique_constraint
+          check_add_unique_constraint(*args)
         when :change_column
           check_change_column(*args)
         when :change_column_default

lib/strong_migrations/checks.rb

@@ -180,6 +180,21 @@ def check_add_reference(method, *args)
       end
     end
 
+    def check_add_unique_constraint(*args)
+      args.extract_options!
+      table, column = args
+
+      # column and using_index cannot be used together
+      # check for column to ensure error message can be generated
+      if column && !new_table?(table)
+        index_name = connection.index_name(table, {column: column})
+        raise_error :add_unique_constraint,
+          index_command: command_str(:add_index, [table, column, {unique: true, algorithm: :concurrently}]),
+          constraint_command: command_str(:add_unique_constraint, [table, {using_index: index_name}]),
+          remove_command: command_str(:remove_unique_constraint, [table, column])
+      end
+    end
+
     def check_change_column(*args)
       options = args.extract_options!
       table, column, type = args

lib/strong_migrations/error_messages.rb

@@ -244,7 +244,24 @@ def change
 Use disable_ddl_transaction! or a separate migration.",
 
     add_exclusion_constraint:
-"Adding an exclusion constraint blocks reads and writes while every row is checked."
+"Adding an exclusion constraint blocks reads and writes while every row is checked.",
+
+    add_unique_constraint:
+"Adding a unique constraint creates a unique index, which blocks reads and writes.
+Instead, create a unique index concurrently, then use it for the constraint.
+
+class %{migration_name} < ActiveRecord::Migration%{migration_suffix}
+  disable_ddl_transaction!
+
+  def up
+    %{index_command}
+    %{constraint_command}
+  end
+
+  def down
+    %{remove_command}
+  end
+end"
   }
   self.enabled_checks = (error_messages.keys - [:remove_index]).map { |k| [k, {}] }.to_h
 end

test/add_unique_constraint_test.rb

@@ -0,0 +1,24 @@
+require_relative "test_helper"
+
+class AddUniqueConstraintTest < Minitest::Test
+  def setup
+    skip unless unique_constraints?
+    super
+  end
+
+  def test_basic
+    assert_unsafe AddUniqueConstraint
+  end
+
+  def test_using_index
+    assert_safe AddUniqueConstraintUsingIndex
+  end
+
+  def test_new_table
+    assert_safe AddUniqueConstraintNewTable
+  end
+
+  def unique_constraints?
+    postgresql? && ActiveRecord::VERSION::STRING.to_f >= 7.1
+  end
+end

test/migrations/add_unique_constraint.rb

@@ -0,0 +1,28 @@
+class AddUniqueConstraint < TestMigration
+  def change
+    add_unique_constraint :users, :name
+  end
+end
+
+class AddUniqueConstraintUsingIndex < TestMigration
+  disable_ddl_transaction!
+
+  def up
+    add_index :users, :name, unique: true, algorithm: :concurrently
+    add_unique_constraint :users, using_index: "index_users_on_name"
+  end
+
+  def down
+    remove_unique_constraint :users, :name
+  end
+end
+
+class AddUniqueConstraintNewTable < TestMigration
+  def change
+    create_table :new_users do |t|
+      t.string :name
+    end
+
+    add_unique_constraint :new_users, :name
+  end
+end