Fixes for PR [#8267]

* Fix Migration#reversible by not using `transaction`.

* Adapt mysql adapter to updated api for remove_column

* Update test after aedcd683684d08eaf30623a4b48ce31a31426372

5 files changed, 24 insertions, 9 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index d37e489f5c..124a2419ec 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -669,10 +669,13 @@ module ActiveRecord
         rename_column_sql
       end
 
-      def remove_column_sql(table_name, *column_names)
-        columns_for_remove(table_name, *column_names).map {|column_name| "DROP #{column_name}" }
+      def remove_column_sql(table_name, column_name, type = nil, options = {})
+        "DROP #{quote_column_name(column_name)}"
+      end
+
+      def remove_columns_sql(table_name, *column_names)
+        column_names.map {|column_name| remove_column_sql(table_name, column_name) }
       end
-      alias :remove_columns_sql :remove_column
 
       def add_index_sql(table_name, column_name, options = {})
         index_name, index_type, index_columns = add_index_options(table_name, column_name, options)
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 273af32237..67339c05e5 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -477,7 +477,7 @@ module ActiveRecord
     #    end
     def reversible
       helper = ReversibleBlockHelper.new(reverting?)
-      transaction{ yield helper }
+      execute_block{ yield helper }
     end
 
     # Runs the given migration classes.
@@ -639,6 +639,15 @@ module ActiveRecord
         "%.3d" % number
       end
     end
+
+    private
+    def execute_block
+      if connection.respond_to? :execute_block
+        super # use normal delegation to record the block
+      else
+        yield
+      end
+    end
   end
 
   # MigrationProxy is used to defer loading of the actual migration classes
diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb
index a6377185a8..79c55045ba 100644
--- a/activerecord/lib/active_record/migration/command_recorder.rb
+++ b/activerecord/lib/active_record/migration/command_recorder.rb
@@ -73,7 +73,7 @@ module ActiveRecord
       [:create_table, :create_join_table, :rename_table, :add_column, :remove_column,
         :rename_index, :rename_column, :add_index, :remove_index, :add_timestamps, :remove_timestamps,
         :change_column_default, :add_reference, :remove_reference, :transaction,
-        :drop_join_table, :drop_table,
+        :drop_join_table, :drop_table, :execute_block,
         :change_column, :execute, :remove_columns, # irreversible methods need to be here too
       ].each do |method|
         class_eval <<-EOV, __FILE__, __LINE__ + 1
@@ -94,6 +94,7 @@ module ActiveRecord
       module StraightReversions
         private
         { transaction:       :transaction,
+          execute_block:     :execute_block,
           create_table:      :drop_table,
           create_join_table: :drop_join_table,
           add_column:        :remove_column,
diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb
index cf4e39c4ef..be59ffc4ab 100644
--- a/activerecord/test/cases/invertible_migration_test.rb
+++ b/activerecord/test/cases/invertible_migration_test.rb
@@ -89,8 +89,10 @@ module ActiveRecord
     end
 
     def teardown
-      if ActiveRecord::Base.connection.table_exists?("horses")
-        ActiveRecord::Base.connection.drop_table("horses")
+      %w[horses new_horses].each do |table|
+        if ActiveRecord::Base.connection.table_exists?(table)
+          ActiveRecord::Base.connection.drop_table(table)
+        end
       end
     end
 
diff --git a/railties/test/application/rake/migrations_test.rb b/railties/test/application/rake/migrations_test.rb
index 0a47fd014c..33c753868c 100644
--- a/railties/test/application/rake/migrations_test.rb
+++ b/railties/test/application/rake/migrations_test.rb
@@ -50,9 +50,9 @@ module ApplicationTests
            assert_match(/AddEmailToUsers: migrated/, output)
 
            output = `rake db:rollback STEP=2`
-           assert_match(/drop_table\("users"\)/, output)
+           assert_match(/drop_table\(:users\)/, output)
            assert_match(/CreateUsers: reverted/, output)
-           assert_match(/remove_column\("users", :email\)/, output)
+           assert_match(/remove_column\(:users, :email, :string\)/, output)
            assert_match(/AddEmailToUsers: reverted/, output)
         end
       end