Merge pull request #31177 from albertoalmagro/remove-default-mysql-engine-from-ar-5-2

Remove default ENGINE=InnoDB for Mysql2 adapter

4 files changed, 87 insertions, 4 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 bff2f51efd..824839c195 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -293,7 +293,7 @@ module ActiveRecord
       end
 
       def create_table(table_name, **options) #:nodoc:
-        super(table_name, options: "ENGINE=InnoDB", **options)
+        super(table_name, options: "", **options)
       end
 
       def bulk_change_table(table_name, operations) #:nodoc:
diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
index bd8c054c28..7ae8073478 100644
--- a/activerecord/lib/active_record/migration/compatibility.rb
+++ b/activerecord/lib/active_record/migration/compatibility.rb
@@ -28,6 +28,14 @@ module ActiveRecord
             super
           end
         end
+
+        def create_table(table_name, options = {})
+          if adapter_name == "Mysql2"
+            super(table_name, options: "ENGINE=InnoDB", **options)
+          else
+            super
+          end
+        end
       end
 
       class V5_0 < V5_1
diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb
index 4e73c557ed..b5613daa72 100644
--- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb
@@ -68,14 +68,14 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase
     def (ActiveRecord::Base.connection).data_source_exists?(*); false; end
 
     %w(SPATIAL FULLTEXT UNIQUE).each do |type|
-      expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name`  (`last_name`)) ENGINE=InnoDB"
+      expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name`  (`last_name`)) "
       actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t|
         t.index :last_name, type: type
       end
       assert_equal expected, actual
     end
 
-    expected = "CREATE TABLE `people` ( INDEX `index_people_on_last_name` USING btree (`last_name`(10))) ENGINE=InnoDB"
+    expected = "CREATE TABLE `people` ( INDEX `index_people_on_last_name` USING btree (`last_name`(10))) "
     actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t|
       t.index :last_name, length: 10, using: :btree
     end
@@ -160,7 +160,7 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase
     ActiveRecord::Base.connection.stubs(:data_source_exists?).with(:temp).returns(false)
     ActiveRecord::Base.connection.stubs(:index_name_exists?).with(:index_temp_on_zip).returns(false)
 
-    expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip`  (`zip`)) ENGINE=InnoDB AS SELECT id, name, zip FROM a_really_complicated_query"
+    expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip`  (`zip`))  AS SELECT id, name, zip FROM a_really_complicated_query"
     actual = ActiveRecord::Base.connection.create_table(:temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query") do |t|
       t.index :zip
     end
diff --git a/activerecord/test/cases/adapters/mysql2/table_options_test.rb b/activerecord/test/cases/adapters/mysql2/table_options_test.rb
index 9f6bd38a8c..1c92df940f 100644
--- a/activerecord/test/cases/adapters/mysql2/table_options_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/table_options_test.rb
@@ -42,3 +42,78 @@ class Mysql2TableOptionsTest < ActiveRecord::Mysql2TestCase
     assert_match %r{COLLATE=utf8mb4_bin}, options
   end
 end
+
+class Mysql2DefaultEngineOptionSchemaDumpTest < ActiveRecord::Mysql2TestCase
+  include SchemaDumpingHelper
+  self.use_transactional_tests = false
+
+  def setup
+    @verbose_was = ActiveRecord::Migration.verbose
+    ActiveRecord::Migration.verbose = false
+  end
+
+  def teardown
+    ActiveRecord::Base.connection.drop_table "mysql_table_options", if_exists: true
+    ActiveRecord::Migration.verbose = @verbose_was
+    ActiveRecord::SchemaMigration.delete_all rescue nil
+  end
+
+  test "schema dump includes ENGINE=InnoDB if not provided" do
+    ActiveRecord::Base.connection.create_table "mysql_table_options", force: true
+
+    output  = dump_table_schema("mysql_table_options")
+    options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
+    assert_match %r{ENGINE=InnoDB}, options
+  end
+
+  test "schema dump includes ENGINE=InnoDB in legacy migrations" do
+    migration = Class.new(ActiveRecord::Migration[5.1]) do
+      def migrate(x)
+        create_table "mysql_table_options", force: true
+      end
+    end.new
+
+    ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+    output  = dump_table_schema("mysql_table_options")
+    options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
+    assert_match %r{ENGINE=InnoDB}, options
+  end
+end
+
+class Mysql2DefaultEngineOptionSqlOutputTest < ActiveRecord::Mysql2TestCase
+  self.use_transactional_tests = false
+
+  def setup
+    @logger_was  = ActiveRecord::Base.logger
+    @log         = StringIO.new
+    @verbose_was = ActiveRecord::Migration.verbose
+    ActiveRecord::Base.logger = ActiveSupport::Logger.new(@log)
+    ActiveRecord::Migration.verbose = false
+  end
+
+  def teardown
+    ActiveRecord::Base.logger       = @logger_was
+    ActiveRecord::Migration.verbose = @verbose_was
+    ActiveRecord::Base.connection.drop_table "mysql_table_options", if_exists: true
+    ActiveRecord::SchemaMigration.delete_all rescue nil
+  end
+
+  test "new migrations do not contain default ENGINE=InnoDB option" do
+    ActiveRecord::Base.connection.create_table "mysql_table_options", force: true
+
+    assert_no_match %r{ENGINE=InnoDB}, @log.string
+  end
+
+  test "legacy migrations contain default ENGINE=InnoDB option" do
+    migration = Class.new(ActiveRecord::Migration[5.1]) do
+      def migrate(x)
+        create_table "mysql_table_options", force: true
+      end
+    end.new
+
+    ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+    assert_match %r{ENGINE=InnoDB}, @log.string
+  end
+end