Added check for add_column with auto-incrementing columns

CHANGELOG.md

@@ -1,5 +1,6 @@
-## 1.7.1 (unreleased)
+## 1.8.0 (unreleased)
 
+- Added check for `add_column` with auto-incrementing columns
 - Updated instructions for removing a column to append to `ignored_columns`
 - Fixed check for adding a column with a default value for MySQL and MariaDB
 

README.md

@@ -64,6 +64,7 @@ Potentially dangerous operations:
 - [renaming a column](#renaming-a-column)
 - [renaming a table](#renaming-a-table)
 - [creating a table with the force option](#creating-a-table-with-the-force-option)
+- [adding an auto-incrementing column](#adding-an-auto-incrementing-column) [unreleased]
 - [adding a stored generated column](#adding-a-stored-generated-column)
 - [adding a check constraint](#adding-a-check-constraint)
 - [executing SQL directly](#executing-SQL-directly)
@@ -256,6 +257,26 @@ end
 
 If you intend to drop an existing table, run `drop_table` first.
 
+### Adding an auto-incrementing column
+
+#### Bad
+
+Adding an auto-incrementing column (`serial`/`bigserial` in Postgres and `AUTO_INCREMENT` in MySQL and MariaDB) causes the entire table to be rewritten. During this time, reads and writes are blocked in Postgres, and writes are blocked in MySQL and MariaDB.
+
+```ruby
+class AddIdToCitiesUsers < ActiveRecord::Migration[7.1]
+  def change
+    add_column :cities_users, :id, :primary_key
+  end
+end
+```
+
+With MySQL and MariaDB, this can also [generate different values on replicas](https://dev.mysql.com/doc/mysql-replication-excerpt/8.0/en/replication-features-auto-increment.html) when using statement-based replication.
+
+#### Good
+
+Create a new table and migrate the data with the same steps as [renaming a table](#renaming-a-table).
+
 ### Adding a stored generated column
 
 #### Bad

lib/strong_migrations/adapters/abstract_adapter.rb

@@ -36,6 +36,10 @@ def rewrite_blocks
         "reads and writes"
       end
 
+      def auto_incrementing_types
+        ["primary_key"]
+      end
+
       private
 
       def connection

lib/strong_migrations/adapters/postgresql_adapter.rb

@@ -169,6 +169,10 @@ def default_volatile?(default)
         rows.empty? || rows.any? { |r| r["provolatile"] == "v" }
       end
 
+      def auto_incrementing_types
+        ["primary_key", "serial", "bigserial"]
+      end
+
       private
 
       def set_timeout(setting, timeout)

lib/strong_migrations/checks.rb

@@ -78,6 +78,10 @@ def check_add_column(*args)
       if type.to_s == "virtual" && options[:stored]
         raise_error :add_column_generated_stored, rewrite_blocks: adapter.rewrite_blocks
       end
+
+      if adapter.auto_incrementing_types.include?(type.to_s)
+        raise_error :add_column_auto_incrementing, rewrite_blocks: adapter.rewrite_blocks
+      end
     end
 
     def check_add_exclusion_constraint(*args)

lib/strong_migrations/error_messages.rb

@@ -53,6 +53,9 @@ def change
     add_column_generated_stored:
 "Adding a stored generated column blocks %{rewrite_blocks} while the entire table is rewritten.",
 
+    add_column_auto_incrementing:
+"Adding an auto-incrementing column blocks %{rewrite_blocks} while the entire table is rewritten.",
+
     change_column:
 "Changing the type of an existing column blocks %{rewrite_blocks}
 while the entire table is rewritten. A safer approach is to:

test/add_column_test.rb

@@ -71,4 +71,18 @@ def test_generated_virtual
     skip if postgresql?
     assert_safe AddColumnGeneratedVirtual
   end
+
+  def test_primary_key
+    assert_unsafe AddColumnPrimaryKey
+  end
+
+  def test_serial
+    skip unless postgresql?
+    assert_unsafe AddColumnSerial
+  end
+
+  def test_bigserial
+    skip unless postgresql?
+    assert_unsafe AddColumnBigserial
+  end
 end

test/migrations/add_column.rb

@@ -52,3 +52,21 @@ def change
     add_column :users, :nice, :virtual, type: :string, as: "LOWER(city)"
   end
 end
+
+class AddColumnPrimaryKey < TestMigration
+  def change
+    add_column :users, :nice, :primary_key
+  end
+end
+
+class AddColumnSerial < TestMigration
+  def change
+    add_column :users, :nice, :serial
+  end
+end
+
+class AddColumnBigserial < TestMigration
+  def change
+    add_column :users, :nice, :bigserial
+  end
+end