Add check for change_column_default

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## master (unreleased)
 
+- Add check for `change_column_default`
 - Add check for `add_unique_key` (Active Record >= 7.1)
 - Add check for `add_column` with stored generated columns
 

README.md

@@ -153,6 +153,10 @@ Potentially dangerous operations:
 - [mismatched reference column types](#mismatched-reference-column-types)
 - [adding a single table inheritance column](#adding-a-single-table-inheritance-column)
 
+Config-specific checks:
+
+- [changing the default value of a column](#changing-the-default-value-of-a-column)
+
 You can also add [custom checks](#custom-checks) or [disable specific checks](#disable-checks).
 
 ### Removing a column
@@ -1171,6 +1175,36 @@ A safer approach is to:
 3. remove the column ignoring from step 1 and apply initial code changes
 4. deploy
 
+### Changing the default value of a column
+
+:x: **Bad**
+
+Active Record < 7 enables partial writes by default, which can cause incorrect values to be inserted when changing the default value of a column.
+
+```ruby
+class ChangeSomeColumnDefault < ActiveRecord::Migration[7.0]
+  def change
+    change_column_default :users, :some_column, from: "old", to: "new"
+  end
+end
+
+User.create!(some_column: "old") # can insert "new"
+```
+
+:white_check_mark: **Good**
+
+Disable partial writes in `config/application.rb`. For Active Record < 7, use:
+
+```ruby
+config.active_record.partial_writes = false
+```
+
+For Active Record 7, use:
+
+```ruby
+config.active_record.partial_inserts = false
+```
+
 ## Assuring Safety
 
 To mark a step in the migration as safe, despite using a method that might otherwise be dangerous, wrap it in a `safety_assured` block.

lib/online_migrations/command_checker.rb

@@ -13,6 +13,7 @@ def initialize(migration)
       @migration = migration
       @safe = false
       @new_tables = []
+      @new_columns = []
       @lock_timeout_checked = false
       @foreign_key_tables = Set.new
       @removed_indexes = []
@@ -116,7 +117,13 @@ def do_check(command, *args, **options, &block)
           check_columns_removal(command, *args, **options)
         else
           if respond_to?(command, true)
-            send(command, *args, **options, &block)
+            if options.any?
+              # Workaround for Active Record < 5 change_column_default
+              # not accepting options.
+              send(command, *args, **options, &block)
+            else
+              send(command, *args, &block)
+            end
           else
             # assume it is safe
             true
@@ -177,6 +184,9 @@ def add_column(table_name, column_name, type, **options)
         default = options[:default]
         volatile_default = false
 
+        # Keep track of new columns for change_column_default check.
+        @new_columns << [table_name.to_s, column_name.to_s]
+
         if !new_or_small_table?(table_name)
           if options.key?(:default) &&
              (postgresql_version < Gem::Version.new("11") || (!default.nil? && (volatile_default = Utils.volatile_default?(connection, type, default))))
@@ -343,6 +353,13 @@ def change_column(table_name, column_name, type, **options)
         end
       end
 
+      def change_column_default(table_name, column_name, _default_or_changes)
+        if Utils.ar_partial_writes? && !new_column?(table_name, column_name)
+          raise_error :change_column_default,
+            config: Utils.ar_partial_writes_setting
+        end
+      end
+
       def change_column_null(table_name, column_name, allow_null, default = nil, **)
         if !allow_null && !new_or_small_table?(table_name)
           safe = false
@@ -416,6 +433,9 @@ def check_columns_removal(command, *args, **options)
       end
 
       def add_timestamps(table_name, **options)
+        @new_columns << [table_name.to_s, "created_at"]
+        @new_columns << [table_name.to_s, "updated_at"]
+
         volatile_default = false
         if !new_or_small_table?(table_name) && !options[:default].nil? &&
            (postgresql_version < Gem::Version.new("11") || (volatile_default = Utils.volatile_default?(connection, :datetime, options[:default])))
@@ -656,6 +676,10 @@ def new_table?(table_name)
         @new_tables.include?(table_name.to_s)
       end
 
+      def new_column?(table_name, column_name)
+        new_table?(table_name) || @new_columns.include?([table_name.to_s, column_name.to_s])
+      end
+
       def small_table?(table_name)
         OnlineMigrations.config.small_tables.include?(table_name.to_s)
       end

lib/online_migrations/error_messages.rb

@@ -248,6 +248,13 @@ def down
 
 6. Deploy",
 
+      change_column_default:
+"Partial writes are enabled, which can cause incorrect values
+to be inserted when changing the default value of a column.
+Disable partial writes in config/application.rb:
+
+config.active_record.<%= config %> = false",
+
       change_column_null:
 "Setting NOT NULL on an existing column blocks reads and writes while every row is checked.
 A safer approach is to add a NOT NULL check constraint and validate it in a separate transaction.

test/command_checker/change_column_default_test.rb

@@ -0,0 +1,85 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+module CommandChecker
+  class ChangeColumnDefaultTest < MiniTest::Test
+    def setup
+      @connection = ActiveRecord::Base.connection
+      @connection.create_table(:users, force: :cascade) do |t|
+        t.string :name
+      end
+    end
+
+    def teardown
+      @connection.drop_table(:users) rescue nil
+    end
+
+    class ChangeColumnDefault < TestMigration
+      def up
+        change_column_default :users, :name, "Test"
+      end
+
+      def down
+        # For Active Record < 5, this change_column_default
+        # is not automatically reversible.
+        change_column_default :users, :name, nil
+      end
+    end
+
+    def test_with_partial_writes
+      with_partial_writes(true) do
+        if ar_version >= 7
+          assert_unsafe ChangeColumnDefault, <<-MSG.strip_heredoc
+            Partial writes are enabled, which can cause incorrect values
+            to be inserted when changing the default value of a column.
+            Disable partial writes in config/application.rb:
+
+            config.active_record.partial_inserts = false
+          MSG
+        else
+          assert_unsafe ChangeColumnDefault, <<-MSG.strip_heredoc
+            config.active_record.partial_writes = false
+          MSG
+        end
+      end
+    end
+
+    class ChangeColumnDefaultHash < TestMigration
+      def change
+        change_column_default :users, :name, from: nil, to: "Test"
+      end
+    end
+
+    def test_with_partial_writes_hash
+      skip("Active Record < 5 does not support :from :to") if ar_version < 5
+
+      with_partial_writes(true) do
+        assert_unsafe ChangeColumnDefaultHash
+      end
+    end
+
+    def test_no_partial_writes
+      with_partial_writes(false) do
+        assert_safe ChangeColumnDefault
+      end
+    end
+
+    class ChangeColumnDefaultNewColumn < TestMigration
+      def up
+        add_column :users, :nice, :boolean
+        change_column_default :users, :nice, true
+      end
+
+      def down
+        remove_column :users, :nice
+      end
+    end
+
+    def test_new_column
+      with_partial_writes(true) do
+        assert_safe ChangeColumnDefaultNewColumn
+      end
+    end
+  end
+end