diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0118ddaca5b86a72ca92e4caac6dc06ebdf513c7..1aa24618beddd20adaefa77e2cf697295680d4ee 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,6 +1,24 @@ +* 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. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index 759d8f523bfa83c1f59b5755a668e81daf144b92..0d3cffd586218458a2a709fda49400049d7b39bb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/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 diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 45242488f87c26e0e319f61adbd891f734c96bac..7e3b225c91ab5a7d6cc3b6365ef8c9609b42dbcf 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/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 = {} diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index fb56e712be64b6dd294f7fae83282a024543092c..3817c8829e4c91d93080713ac1e3b8c62a476760 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -13,9 +13,9 @@ def column_spec(column) end def column_spec_for_primary_key(column) - return {} if default_primary_key?(column) - spec = { id: schema_type(column).inspect } - spec.merge!(prepare_column_options(column).except!(:null, :comment)) + spec = {} + spec[:id] = schema_type(column).inspect unless default_primary_key?(column) + spec.merge!(prepare_column_options(column).except!(:null)) spec[:default] ||= "nil" if explicit_primary_key_default?(column) spec end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 2634bc5b2b9689b8093ff15e8eeaec82d04b47d5..de0b50504bc116e0d62e6518dc083be30a41aa86 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -295,13 +295,16 @@ 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) + if id.is_a?(Hash) + options.merge!(id.except(:type)) + id = id.fetch(:type, :primary_key) + end + if pk.is_a?(Array) td.primary_keys pk else @@ -1379,14 +1382,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( 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 8925a96b8a49eded7e0bde6104d7982c5ab2fca5..3435e2884ec801c34fc7c6643c43ccba9bdd557b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/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=(?\w+)(?: COLLATE=(?\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 diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb index 8e5351d14b8831283da973a614fd796a5cf7c192..3b91e15b2c76e97007d99a456fba4138c5eb6fef 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb +++ b/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) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index d21535a7099fb29bc1502122383cfd21fdcb159b..52a8a0b97d6f1f4d4e5e5a9ffc929f105f688898 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/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 diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 9447e2e8d8b8207fb14a860e2e2f8fdae97a8180..af5683d23f0244e0b3c2fb40e3a7170d6a53bd47 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/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) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 1a6b9b04fedd07fb7ec2ffa79a8fbbf30193c1d6..ba60ab1d67018ecd6c5b0b95c0588755290a1333 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/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) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index 824e9372b93a667cab68a8bd895864cde932a201..558cd649ef52a6efa127b420a193981246fc8305 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/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) diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 953b534f2db9fe8e8769b923a077f6c330615056..7a680225531b77c53ca14f296bac2efbf32f53b3 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -125,7 +125,10 @@ def table(table, stream) tbl.print ", primary_key: #{pk.inspect}" unless pk == "id" pkcol = columns.detect { |c| c.name == pk } pkcolspec = column_spec_for_primary_key(pkcol) - if pkcolspec.present? + unless pkcolspec.empty? + if pkcolspec != pkcolspec.slice(:id, :default) + pkcolspec = { id: { type: pkcolspec.delete(:id), **pkcolspec }.compact } + end tbl.print ", #{format_colspec(pkcolspec)}" end when Array @@ -240,7 +243,9 @@ def foreign_keys(table, stream) end def format_colspec(colspec) - colspec.map { |key, value| "#{key}: #{value}" }.join(", ") + colspec.map do |key, value| + "#{key}: #{ value.is_a?(Hash) ? "{ #{format_colspec(value)} }" : value }" + end.join(", ") end def format_options(options) diff --git a/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb b/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb index 0bdbefdfb9f55ff1955bdaf9ce8520cb8c06e293..787206deb950e1557e0a4721ecbea08f43778c3b 100644 --- a/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb +++ b/activerecord/test/cases/adapters/mysql2/charset_collation_test.rb @@ -9,7 +9,7 @@ class Mysql2CharsetCollationTest < ActiveRecord::Mysql2TestCase setup do @connection = ActiveRecord::Base.connection - @connection.create_table :charset_collations, force: true do |t| + @connection.create_table :charset_collations, id: { type: :string, collation: "utf8mb4_bin" }, force: true do |t| t.string :string_ascii_bin, charset: "ascii", collation: "ascii_bin" t.text :text_ucs2_unicode_ci, charset: "ucs2", collation: "ucs2_unicode_ci" end @@ -50,6 +50,7 @@ class Mysql2CharsetCollationTest < ActiveRecord::Mysql2TestCase test "schema dump includes collation" do output = dump_table_schema("charset_collations") + assert_match %r/create_table "charset_collations", id: { type: :string, collation: "utf8mb4_bin" }/, output assert_match %r{t\.string\s+"string_ascii_bin",\s+collation: "ascii_bin"$}, output assert_match %r{t\.text\s+"text_ucs2_unicode_ci",\s+collation: "ucs2_unicode_ci"$}, output end diff --git a/activerecord/test/cases/adapters/mysql2/table_options_test.rb b/activerecord/test/cases/adapters/mysql2/table_options_test.rb index b91f4ef1371a504aa8898f7a309cf060219e3420..a47413da383c8c126619b2aef379c11a156a743d 100644 --- a/activerecord/test/cases/adapters/mysql2/table_options_test.rb +++ b/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: "(?.*)"}.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: "(?.*)"}.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: "(?.*)"}.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: "(?.*)"}.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: "(?.*)"}.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: "(?.*)"}.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 diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 5526f1b91b7b0032856c9c4a41a7bd0d8e298f05..220e4e6915a465c57d4ecc7b11710450fec6e355 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -39,7 +39,7 @@ class PkCommented < ActiveRecord::Base end @connection.create_table("pk_commenteds", comment: "Table comment", id: false, force: true) do |t| - t.integer :id, comment: "Primary key comment", primary_key: true + t.primary_key :id, comment: "Primary key comment" end Commented.reset_column_information @@ -197,8 +197,7 @@ def test_comment_on_primary_key def test_schema_dump_with_primary_key_comment output = dump_table_schema "pk_commenteds" - assert_match %r[create_table "pk_commenteds",.*\s+comment: "Table comment"], output - assert_no_match %r[create_table "pk_commenteds",.*\s+comment: "Primary key comment"], output + assert_match %r[create_table "pk_commenteds", id: { comment: "Primary key comment" }.*, comment: "Table comment"], output end end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index faebbbd39cf3cd6193c25656203b46232a32d867..1ee8842fc6f3ed5f2ca2b86935f1e2076a0a7766 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -312,7 +312,7 @@ def test_any_type_primary_key test "schema dump primary key includes type and options" do schema = dump_table_schema "barcodes" - assert_match %r{create_table "barcodes", primary_key: "code", id: :string, limit: 42}, schema + assert_match %r/create_table "barcodes", primary_key: "code", id: { type: :string, limit: 42 }/, schema assert_no_match %r{t\.index \["code"\]}, schema end @@ -320,7 +320,7 @@ def test_any_type_primary_key test "schema typed primary key column" do @connection.create_table(:scheduled_logs, id: :timestamp, precision: 6, force: true) schema = dump_table_schema("scheduled_logs") - assert_match %r/create_table "scheduled_logs", id: :timestamp, precision: 6/, schema + assert_match %r/create_table "scheduled_logs", id: { type: :timestamp, precision: 6.* }/, schema end end end @@ -462,7 +462,7 @@ class Widget < ActiveRecord::Base assert_predicate column, :unsigned? schema = dump_table_schema "widgets" - assert_match %r{create_table "widgets", id: :integer, unsigned: true, }, schema + assert_match %r/create_table "widgets", id: { type: :integer, unsigned: true }/, schema end test "bigint primary key with unsigned" do @@ -474,7 +474,7 @@ class Widget < ActiveRecord::Base assert_predicate column, :unsigned? schema = dump_table_schema "widgets" - assert_match %r{create_table "widgets", id: :bigint, unsigned: true, }, schema + assert_match %r/create_table "widgets", id: { type: :bigint, unsigned: true }/, schema end end end