Add support for bulk change_table revert

Co-authored-by: Eugene Kenny <elkenny@gmail.com>

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -1461,6 +1461,31 @@ def schema_creation # :nodoc:
         SchemaCreation.new(self)
       end
 
+      def bulk_change_table(table_name, operations) # :nodoc:
+        sql_fragments = []
+        non_combinable_operations = []
+
+        operations.each do |command, args|
+          table, arguments = args.shift, args
+          method = :"#{command}_for_alter"
+
+          if respond_to?(method, true)
+            sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) }
+            sql_fragments.concat(sqls)
+            non_combinable_operations.concat(procs)
+          else
+            execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
+            non_combinable_operations.each(&:call)
+            sql_fragments = []
+            non_combinable_operations = []
+            send(command, table, *arguments)
+          end
+        end
+
+        execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
+        non_combinable_operations.each(&:call)
+      end
+
       private
         def validate_change_column_null_argument!(value)
           unless value == true || value == false
@@ -1694,31 +1719,6 @@ def reference_name_for_table(table_name)
           table_name.to_s.singularize
         end
 
-        def bulk_change_table(table_name, operations)
-          sql_fragments = []
-          non_combinable_operations = []
-
-          operations.each do |command, args|
-            table, arguments = args.shift, args
-            method = :"#{command}_for_alter"
-
-            if respond_to?(method, true)
-              sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) }
-              sql_fragments.concat(sqls)
-              non_combinable_operations.concat(procs)
-            else
-              execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
-              non_combinable_operations.each(&:call)
-              sql_fragments = []
-              non_combinable_operations = []
-              send(command, table, *arguments)
-            end
-          end
-
-          execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
-          non_combinable_operations.each(&:call)
-        end
-
         def add_column_for_alter(table_name, column_name, type, **options)
           td = create_table_definition(table_name)
           cd = td.new_column_definition(column_name, type, **options)

activerecord/lib/active_record/migration/command_recorder.rb

@@ -124,7 +124,15 @@ def #{method}(*args, &block)          # def create_table(*args, &block)
       alias :remove_belongs_to :remove_reference
 
       def change_table(table_name, **options) # :nodoc:
-        yield delegate.update_table_definition(table_name, self)
+        if delegate.supports_bulk_alter? && options[:bulk]
+          recorder = self.class.new(self.delegate)
+          recorder.reverting = @reverting
+          yield recorder.delegate.update_table_definition(table_name, recorder)
+          commands = recorder.commands
+          @commands << [:change_table, [table_name], -> t { bulk_change_table(table_name, commands) }]
+        else
+          yield delegate.update_table_definition(table_name, self)
+        end
       end
 
       def replay(migration)

activerecord/test/cases/migration/command_recorder_test.rb

@@ -120,6 +120,30 @@ def test_invert_change_table
         end
       end
 
+      if ActiveRecord::Base.connection.supports_bulk_alter?
+        def test_bulk_invert_change_table
+          block = Proc.new do |t|
+            t.string :name
+            t.rename :kind, :cultivar
+          end
+
+          @recorder.revert do
+            @recorder.change_table :fruits, bulk: true, &block
+          end
+
+          @recorder.revert do
+            @recorder.revert do
+              @recorder.change_table :fruits, bulk: true, &block
+            end
+          end
+
+          assert_equal [
+            [:change_table, [:fruits]],
+            [:change_table, [:fruits]]
+          ], @recorder.commands.map { |command| command[0...-1] }
+        end
+      end
+
       def test_invert_create_table
         @recorder.revert do
           @recorder.record :create_table, [:system_settings]

activerecord/test/cases/migration_test.rb

@@ -1561,6 +1561,47 @@ def indexes
         @indexes ||= Person.connection.indexes("delete_me")
       end
   end # AlterTableMigrationsTest
+
+  class RevertBulkAlterTableMigrationsTest < ActiveRecord::TestCase
+    self.use_transactional_tests = false
+
+    def setup
+      @connection = Person.connection
+      Person.reset_column_information
+      Person.reset_sequence_name
+    end
+
+    teardown do
+      @connection.remove_columns(:people, :column1, :column2) rescue nil
+    end
+
+    def test_bulk_revert
+      @connection.add_column(:people, :column1, :string)
+      @connection.add_column(:people, :column2, :string)
+      assert_column Person, :column1
+      assert_column Person, :column2
+
+      migration = Class.new(ActiveRecord::Migration::Current) {
+        disable_ddl_transaction!
+
+        def write(text = ""); end
+
+        def change
+          change_table :people, bulk: true do |t|
+            t.column :column1, :string
+            t.column :column2, :string
+          end
+        end
+      }.new
+
+      assert_queries(1) do
+        migration.migrate(:down)
+      end
+
+      assert_no_column Person, :column1
+      assert_no_column Person, :column2
+    end
+  end
 end
 
 class CopyMigrationsTest < ActiveRecord::TestCase