Default engine `ENGINE=InnoDB` is no longer dumped to make schema more agnostic

5 years ago, I made dumping full table options at #17569, especially to
dump `ENGINE=InnoDB ROW_FORMAT=DYNAMIC` to use utf8mb4 with large key
prefix.

In that time, omitting the default engine `ENGINE=InnoDB` was not useful
since `ROW_FORMAT=DYNAMIC` always remains as long as using utf8mb4 with
large key prefix.

But now, MySQL 5.7.9 has finally changed the default row format to
DYNAMIC, utf8mb4 with large key prefix can be used without dumping the
default engine and the row format explicitly.

So now is a good time to make the default engine is omitted.

Before:

```ruby
create_table "accounts", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci", force: :cascade do |t|
end
```

After:

```ruby
create_table "accounts", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
end
```

To entirely omit `:options` option to make schema agnostic, I've added
`:charset` and `:collation` table options to exclude `CHARSET` and
`COLLATE` from `:options`.

Fixes #26209.
Closes #29472.

See also #33608, #33853, and #34742.

activerecord/CHANGELOG.md

+*   Default engine `ENGINE=InnoDB` is no longer dumped to make schema more agnostic.
+
+    Before:
+
+    ```ruby
+    create_table "accounts", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci", force: :cascade do |t|
+    end
+    ```
+
+    After:
+
+    ```ruby
+    create_table "accounts", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
+    end
+    ```
+
+    *Ryuta Kamizono*
+
 *   Added delegated type as an alternative to single-table inheritance for representing class hierarchies.
     See ActiveRecord::DelegatedType for the full description.
-    
+
     *DHH*
 
 *   Deprecate aggregations with group by duplicated fields.

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

@@ -53,7 +53,7 @@ def visit_TableDefinition(o)
           end
 
           create_sql << "(#{statements.join(', ')})" if statements.present?
-          add_table_options!(create_sql, table_options(o))
+          add_table_options!(create_sql, o)
           create_sql << " AS #{to_sql(o.as)}" if o.as
           create_sql
         end
@@ -106,17 +106,8 @@ def supports_index_using?
           true
         end
 
-        def table_options(o)
-          table_options = {}
-          table_options[:comment] = o.comment
-          table_options[:options] = o.options
-          table_options
-        end
-
-        def add_table_options!(create_sql, options)
-          if options_sql = options[:options]
-            create_sql << " #{options_sql}"
-          end
+        def add_table_options!(create_sql, o)
+          create_sql << " #{o.options}" if o.options
           create_sql
         end
 

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

@@ -277,7 +277,8 @@ def initialize(
         if_not_exists: false,
         options: nil,
         as: nil,
-        comment: nil
+        comment: nil,
+        **
       )
         @conn = conn
         @columns_hash = {}

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

@@ -295,9 +295,7 @@ def primary_key(table_name)
       #
       # See also TableDefinition#column for details on how to create columns.
       def create_table(table_name, id: :primary_key, primary_key: nil, force: nil, **options)
-        td = create_table_definition(
-          table_name, **options.extract!(:temporary, :if_not_exists, :options, :as, :comment)
-        )
+        td = create_table_definition(table_name, **extract_table_options!(options))
 
         if id && !td.as
           pk = primary_key || Base.get_primary_key(table_name.to_s.singularize)
@@ -1379,14 +1377,18 @@ def schema_creation
           SchemaCreation.new(self)
         end
 
-        def create_table_definition(*args, **options)
-          TableDefinition.new(self, *args, **options)
+        def create_table_definition(name, **options)
+          TableDefinition.new(self, name, **options)
         end
 
         def create_alter_table(name)
           AlterTable.new create_table_definition(name)
         end
 
+        def extract_table_options!(options)
+          options.extract!(:temporary, :if_not_exists, :options, :as, :comment, :charset, :collation)
+        end
+
         def fetch_type_metadata(sql_type)
           cast_type = lookup_cast_type(sql_type)
           SqlTypeMetadata.new(

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -414,24 +414,31 @@ def foreign_keys(table_name)
       end
 
       def table_options(table_name) # :nodoc:
-        table_options = {}
-
         create_table_info = create_table_info(table_name)
 
         # strip create_definitions and partition_options
         # Be aware that `create_table_info` might not include any table options due to `NO_TABLE_OPTIONS` sql mode.
         raw_table_options = create_table_info.sub(/\A.*\n\) ?/m, "").sub(/\n\/\*!.*\*\/\n\z/m, "").strip
 
+        return if raw_table_options.empty?
+
+        table_options = {}
+
+        if / DEFAULT CHARSET=(?<charset>\w+)(?: COLLATE=(?<collation>\w+))?/ =~ raw_table_options
+          raw_table_options = $` + $' # before part + after part
+          table_options[:charset] = charset
+          table_options[:collation] = collation if collation
+        end
+
         # strip AUTO_INCREMENT
         raw_table_options.sub!(/(ENGINE=\w+)(?: AUTO_INCREMENT=\d+)/, '\1')
 
-        table_options[:options] = raw_table_options unless raw_table_options.blank?
-
         # strip COMMENT
         if raw_table_options.sub!(/ COMMENT='.+'/, "")
           table_options[:comment] = table_comment(table_name)
         end
 
+        table_options[:options] = raw_table_options unless raw_table_options == "ENGINE=InnoDB"
         table_options
       end
 

activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb

@@ -40,8 +40,11 @@ def visit_IndexDefinition(o, create = false)
             add_sql_comment!(sql.join(" "), o.comment)
           end
 
-          def add_table_options!(create_sql, options)
-            add_sql_comment!(super, options[:comment])
+          def add_table_options!(create_sql, o)
+            create_sql = super
+            create_sql << " DEFAULT CHARSET=#{o.charset}" if o.charset
+            create_sql << " COLLATE=#{o.collation}" if o.collation
+            add_sql_comment!(create_sql, o.comment)
           end
 
           def add_column_options!(sql, options)

activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb

@@ -60,6 +60,14 @@ module ColumnMethods
       class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
         include ColumnMethods
 
+        attr_reader :charset, :collation
+
+        def initialize(conn, name, charset: nil, collation: nil, **)
+          super
+          @charset = charset
+          @collation = collation
+        end
+
         def new_column_definition(name, type, **options) # :nodoc:
           case type
           when :virtual

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

@@ -154,8 +154,8 @@ def schema_creation
             MySQL::SchemaCreation.new(self)
           end
 
-          def create_table_definition(*args, **options)
-            MySQL::TableDefinition.new(self, *args, **options)
+          def create_table_definition(name, **options)
+            MySQL::TableDefinition.new(self, name, **options)
           end
 
           def new_column_from_field(table_name, field)

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

@@ -621,8 +621,8 @@ def schema_creation
             PostgreSQL::SchemaCreation.new(self)
           end
 
-          def create_table_definition(*args, **options)
-            PostgreSQL::TableDefinition.new(self, *args, **options)
+          def create_table_definition(name, **options)
+            PostgreSQL::TableDefinition.new(self, name, **options)
           end
 
           def create_alter_table(name)

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

@@ -87,8 +87,8 @@ def schema_creation
             SQLite3::SchemaCreation.new(self)
           end
 
-          def create_table_definition(*args, **options)
-            SQLite3::TableDefinition.new(self, *args, **options)
+          def create_table_definition(name, **options)
+            SQLite3::TableDefinition.new(self, name, **options)
           end
 
           def validate_index_length!(table_name, new_name, internal = false)

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

@@ -5,6 +5,7 @@
 
 class Mysql2TableOptionsTest < ActiveRecord::Mysql2TestCase
   include SchemaDumpingHelper
+  self.use_transactional_tests = false
 
   def setup
     @connection = ActiveRecord::Base.connection
@@ -17,29 +18,36 @@ def teardown
   test "table options with ENGINE" do
     @connection.create_table "mysql_table_options", force: true, options: "ENGINE=MyISAM"
     output = dump_table_schema("mysql_table_options")
-    options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
-    assert_match %r{ENGINE=MyISAM}, options
+    expected = /create_table "mysql_table_options", charset: "utf8mb4"(?:, collation: "\w+")?, options: "ENGINE=MyISAM", force: :cascade/
+    assert_match expected, output
   end
 
   test "table options with ROW_FORMAT" do
     @connection.create_table "mysql_table_options", force: true, options: "ROW_FORMAT=REDUNDANT"
     output = dump_table_schema("mysql_table_options")
-    options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
-    assert_match %r{ROW_FORMAT=REDUNDANT}, options
+    expected = /create_table "mysql_table_options", charset: "utf8mb4"(?:, collation: "\w+")?, options: "ENGINE=InnoDB ROW_FORMAT=REDUNDANT", force: :cascade/
+    assert_match expected, output
   end
 
   test "table options with CHARSET" do
-    @connection.create_table "mysql_table_options", force: true, options: "CHARSET=utf8mb4"
+    @connection.create_table "mysql_table_options", force: true, options: "CHARSET=latin1"
     output = dump_table_schema("mysql_table_options")
-    options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
-    assert_match %r{CHARSET=utf8mb4}, options
+    expected = /create_table "mysql_table_options", charset: "latin1", force: :cascade/
+    assert_match expected, output
   end
 
   test "table options with COLLATE" do
     @connection.create_table "mysql_table_options", force: true, options: "COLLATE=utf8mb4_bin"
     output = dump_table_schema("mysql_table_options")
-    options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
-    assert_match %r{COLLATE=utf8mb4_bin}, options
+    expected = /create_table "mysql_table_options", charset: "utf8mb4", collation: "utf8mb4_bin", force: :cascade/
+    assert_match expected, output
+  end
+
+  test "charset and collation options" do
+    @connection.create_table "mysql_table_options", force: true, charset: "utf8mb4", collation: "utf8mb4_bin"
+    output = dump_table_schema("mysql_table_options")
+    expected = /create_table "mysql_table_options", charset: "utf8mb4", collation: "utf8mb4_bin", force: :cascade/
+    assert_match expected, output
   end
 
   test "schema dump works with NO_TABLE_OPTIONS sql mode" do
@@ -60,47 +68,10 @@ def teardown
   end
 end
 
-class Mysql2DefaultEngineOptionSchemaDumpTest < ActiveRecord::Mysql2TestCase
+class Mysql2DefaultEngineOptionTest < 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], ActiveRecord::Base.connection.schema_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
@@ -120,6 +91,10 @@ def teardown
     ActiveRecord::Base.connection.create_table "mysql_table_options", force: true
 
     assert_no_match %r{ENGINE=InnoDB}, @log.string
+
+    output = dump_table_schema("mysql_table_options")
+    expected = /create_table "mysql_table_options", charset: "utf8mb4"(?:, collation: "\w+")?, force: :cascade/
+    assert_match expected, output
   end
 
   test "legacy migrations contain default ENGINE=InnoDB option" do
@@ -132,5 +107,9 @@ def migrate(x)
     ActiveRecord::Migrator.new(:up, [migration], ActiveRecord::Base.connection.schema_migration).migrate
 
     assert_match %r{ENGINE=InnoDB}, @log.string
+
+    output = dump_table_schema("mysql_table_options")
+    expected = /create_table "mysql_table_options", charset: "utf8mb4"(?:, collation: "\w+")?, force: :cascade/
+    assert_match expected, output
   end
 end