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

Remove default ENGINE=InnoDB for Mysql2 adapter

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -293,7 +293,7 @@ def table_comment(table_name) # :nodoc:
       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:

activerecord/lib/active_record/migration/compatibility.rb

@@ -28,6 +28,14 @@ def change_column(table_name, column_name, type, options = {})
             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

activerecord/test/cases/adapters/mysql2/active_schema_test.rb

@@ -68,14 +68,14 @@ def test_index_in_create
     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 @@ def test_indexes_in_create
     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

activerecord/test/cases/adapters/mysql2/table_options_test.rb

@@ -42,3 +42,78 @@ def teardown
     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

guides/source/active_record_migrations.md

@@ -353,8 +353,7 @@ create_table :products, options: "ENGINE=BLACKHOLE" do |t|
 end
 ```
 
-will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table
-(when using MySQL or MariaDB, the default is `ENGINE=InnoDB`).
+will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table.
 
 Also you can pass the `:comment` option with any description for the table
 that will be stored in database itself and can be viewed with database administration