Remove default ENGINE=InnoDB for Mysql2 adapter

Before this commit ENGINE=InnoDB was added by default to Mysql2 adapter
+create_table+ if no +options+ option was provided. This default ENGINE
was lost as soon as something was passed in at +options+ option, making
its goal and propagation inconsistent, as the programmer needed to
remember including ENGINE=InnoDB when something was passed in.
This commit removes default ENGINE as its use isn't needed anymore for
current MySQL and MariaDB versions. It adds compatibility support and
tests to ensure that default ENGINE is still present for migrations
with version 5.1 and before. It also ensures we still dump the ENGINE
option to +schema.rb+ in order to avoid inconsistencies.

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 479131caad..2d1d265f65 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