From 1745e905a314754b65eabc5fa28671c80796f09f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 25 Jan 2019 22:01:07 +0900 Subject: [PATCH] Allow changing text and blob size without giving the `limit` option In MySQL, the text column size is 65,535 bytes by default (1 GiB in PostgreSQL). It is sometimes too short when people want to use a text column, so they sometimes change the text size to mediumtext (16 MiB) or longtext (4 GiB) by giving the `limit` option. Unlike MySQL, PostgreSQL doesn't allow the `limit` option for a text column (raises ERROR: type modifier is not allowed for type "text"). So `limit: 4294967295` (longtext) couldn't be used in Action Text. I've allowed changing text and blob size without giving the `limit` option, it prevents that migration failure on PostgreSQL. --- ...0917164000_create_action_mailbox_tables.rb | 6 +---- ...180528164100_create_action_text_tables.rb} | 2 +- ...180528164100_create_action_text_tables.rb} | 5 ++-- actiontext/test/dummy/db/schema.rb | 2 +- activerecord/CHANGELOG.md | 4 ++++ .../abstract_mysql_adapter.rb | 5 ++-- .../mysql/schema_definitions.rb | 10 ++++++++ .../mysql/schema_dumper.rb | 10 ++++++-- activerecord/test/cases/migration_test.rb | 12 ++++++++++ activerecord/test/cases/schema_dumper_test.rb | 24 ++++++++++++------- .../test/schema/mysql2_specific_schema.rb | 8 +++++++ 11 files changed, 65 insertions(+), 23 deletions(-) rename actiontext/{test/dummy/db/migrate/2018052816_create_action_text_tables.rb => db/migrate/20180528164100_create_action_text_tables.rb} (90%) rename actiontext/{db/migrate/201805281641_create_action_text_tables.rb => test/dummy/db/migrate/20180528164100_create_action_text_tables.rb} (74%) diff --git a/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb b/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb index 89ab66c1a9..2bf4335808 100644 --- a/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb +++ b/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb @@ -5,11 +5,7 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0] t.string :message_id, null: false t.string :message_checksum, null: false - if supports_datetime_with_precision? - t.timestamps precision: 6 - else - t.timestamps - end + t.timestamps t.index [ :message_id, :message_checksum ], name: "index_action_mailbox_inbound_emails_uniqueness", unique: true end diff --git a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb b/actiontext/db/migrate/20180528164100_create_action_text_tables.rb similarity index 90% rename from actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb rename to actiontext/db/migrate/20180528164100_create_action_text_tables.rb index 6e7177620f..e7c66ea6ae 100644 --- a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb +++ b/actiontext/db/migrate/20180528164100_create_action_text_tables.rb @@ -2,7 +2,7 @@ class CreateActionTextTables < ActiveRecord::Migration[6.0] def change create_table :action_text_rich_texts do |t| t.string :name, null: false - t.text :body, limit: 16777215 + t.text :body, size: :long t.references :record, null: false, polymorphic: true, index: false t.timestamps diff --git a/actiontext/db/migrate/201805281641_create_action_text_tables.rb b/actiontext/test/dummy/db/migrate/20180528164100_create_action_text_tables.rb similarity index 74% rename from actiontext/db/migrate/201805281641_create_action_text_tables.rb rename to actiontext/test/dummy/db/migrate/20180528164100_create_action_text_tables.rb index 74c7a0ecb9..e7c66ea6ae 100644 --- a/actiontext/db/migrate/201805281641_create_action_text_tables.rb +++ b/actiontext/test/dummy/db/migrate/20180528164100_create_action_text_tables.rb @@ -2,11 +2,10 @@ class CreateActionTextTables < ActiveRecord::Migration[6.0] def change create_table :action_text_rich_texts do |t| t.string :name, null: false - t.text :body, limit: 16777215 + t.text :body, size: :long t.references :record, null: false, polymorphic: true, index: false - t.datetime :created_at, null: false - t.datetime :updated_at, null: false + t.timestamps t.index [ :record_type, :record_id, :name ], name: "index_action_text_rich_texts_uniqueness", unique: true end diff --git a/actiontext/test/dummy/db/schema.rb b/actiontext/test/dummy/db/schema.rb index 71080a1c69..5216c5fd33 100644 --- a/actiontext/test/dummy/db/schema.rb +++ b/actiontext/test/dummy/db/schema.rb @@ -14,7 +14,7 @@ ActiveRecord::Schema.define(version: 2018_10_03_185713) do create_table "action_text_rich_texts", force: :cascade do |t| t.string "name", null: false - t.text "body", limit: 16777215 + t.text "body" t.string "record_type", null: false t.integer "record_id", null: false t.datetime "created_at", precision: 6, null: false diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 167ade30e3..67cf5881d3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* MySQL: Support `:size` option to change text and blob size. + + *Ryuta Kamizono* + * Make `t.timestamps` with precision by default. *Ryuta Kamizono* 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 415a0576c4..7b69a63f6e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -29,7 +29,7 @@ module ActiveRecord NATIVE_DATABASE_TYPES = { primary_key: "bigint auto_increment PRIMARY KEY", string: { name: "varchar", limit: 255 }, - text: { name: "text", limit: 65535 }, + text: { name: "text" }, integer: { name: "int", limit: 4 }, float: { name: "float", limit: 24 }, decimal: { name: "decimal" }, @@ -37,7 +37,8 @@ module ActiveRecord timestamp: { name: "timestamp" }, time: { name: "time" }, date: { name: "date" }, - binary: { name: "blob", limit: 65535 }, + binary: { name: "blob" }, + blob: { name: "blob" }, boolean: { name: "tinyint", limit: 1 }, json: { name: "json" }, } 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 2ed4ad16ae..90bcdf3297 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -56,6 +56,16 @@ module ActiveRecord case type when :virtual type = options[:type] + when :text, :blob, :binary + case (size = options[:size])&.to_s + when "tiny", "medium", "long" + sql_type = @conn.native_database_types[type][:name] + type = "#{size}#{sql_type}" + else + raise ArgumentError, <<~MSG unless size.nil? + #{size.inspect} is invalid :size value. Only :tiny, :medium, and :long are allowed. + MSG + end when :primary_key type = :integer options[:limit] ||= 8 diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index d23178e43c..57518b02fa 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -10,6 +10,10 @@ module ActiveRecord spec[:unsigned] = "true" if column.unsigned? spec[:auto_increment] = "true" if column.auto_increment? + if /\A(?tiny|medium|long)(?:text|blob)/ =~ column.sql_type + spec = { size: size.to_sym.inspect }.merge!(spec) + end + if @connection.supports_virtual_columns? && column.virtual? spec[:as] = extract_expression_for_virtual_column(column) spec[:stored] = "true" if /\b(?:STORED|PERSISTENT)\b/.match?(column.extra) @@ -37,13 +41,15 @@ module ActiveRecord case column.sql_type when /\Atimestamp\b/ :timestamp - when "tinyblob" - :blob else super end end + def schema_limit(column) + super unless /\A(?:tiny|medium|long)?(?:text|blob)/.match?(column.sql_type) + end + def schema_precision(column) super unless /\A(?:date)?time(?:stamp)?\b/.match?(column.sql_type) && column.precision == 0 end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 46e2ff79d9..02031e51ef 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -626,6 +626,18 @@ class MigrationTest < ActiveRecord::TestCase ensure Person.connection.drop_table :test_text_limits, if_exists: true end + + def test_invalid_text_size_should_raise + e = assert_raise(ArgumentError) do + Person.connection.create_table :test_text_sizes, force: true do |t| + t.text :bigtext, size: 0xfffffffff + end + end + + assert_match(/#{0xfffffffff} is invalid :size value\. Only :tiny, :medium, and :long are allowed\./, e.message) + ensure + Person.connection.drop_table :test_text_sizes, if_exists: true + end end if ActiveRecord::Base.connection.supports_advisory_locks? diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index dda3efa47c..49e9be9565 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -245,25 +245,31 @@ class SchemaDumperTest < ActiveRecord::TestCase if current_adapter?(:Mysql2Adapter) def test_schema_dump_includes_length_for_mysql_binary_fields - output = standard_dump + output = dump_table_schema "binary_fields" assert_match %r{t\.binary\s+"var_binary",\s+limit: 255$}, output assert_match %r{t\.binary\s+"var_binary_large",\s+limit: 4095$}, output end def test_schema_dump_includes_length_for_mysql_blob_and_text_fields - output = standard_dump - assert_match %r{t\.blob\s+"tiny_blob",\s+limit: 255$}, output + output = dump_table_schema "binary_fields" + assert_match %r{t\.binary\s+"tiny_blob",\s+size: :tiny$}, output assert_match %r{t\.binary\s+"normal_blob"$}, output - assert_match %r{t\.binary\s+"medium_blob",\s+limit: 16777215$}, output - assert_match %r{t\.binary\s+"long_blob",\s+limit: 4294967295$}, output - assert_match %r{t\.text\s+"tiny_text",\s+limit: 255$}, output + assert_match %r{t\.binary\s+"medium_blob",\s+size: :medium$}, output + assert_match %r{t\.binary\s+"long_blob",\s+size: :long$}, output + assert_match %r{t\.text\s+"tiny_text",\s+size: :tiny$}, output assert_match %r{t\.text\s+"normal_text"$}, output - assert_match %r{t\.text\s+"medium_text",\s+limit: 16777215$}, output - assert_match %r{t\.text\s+"long_text",\s+limit: 4294967295$}, output + assert_match %r{t\.text\s+"medium_text",\s+size: :medium$}, output + assert_match %r{t\.text\s+"long_text",\s+size: :long$}, output + assert_match %r{t\.binary\s+"tiny_blob_2",\s+size: :tiny$}, output + assert_match %r{t\.binary\s+"medium_blob_2",\s+size: :medium$}, output + assert_match %r{t\.binary\s+"long_blob_2",\s+size: :long$}, output + assert_match %r{t\.text\s+"tiny_text_2",\s+size: :tiny$}, output + assert_match %r{t\.text\s+"medium_text_2",\s+size: :medium$}, output + assert_match %r{t\.text\s+"long_text_2",\s+size: :long$}, output end def test_schema_does_not_include_limit_for_emulated_mysql_boolean_fields - output = standard_dump + output = dump_table_schema "booleans" assert_no_match %r{t\.boolean\s+"has_fun",.+limit: 1}, output end diff --git a/activerecord/test/schema/mysql2_specific_schema.rb b/activerecord/test/schema/mysql2_specific_schema.rb index 61e9bc9af7..b143035213 100644 --- a/activerecord/test/schema/mysql2_specific_schema.rb +++ b/activerecord/test/schema/mysql2_specific_schema.rb @@ -27,6 +27,7 @@ ActiveRecord::Schema.define do create_table :binary_fields, force: true do |t| t.binary :var_binary, limit: 255 t.binary :var_binary_large, limit: 4095 + t.tinyblob :tiny_blob t.blob :normal_blob t.mediumblob :medium_blob @@ -36,6 +37,13 @@ ActiveRecord::Schema.define do t.mediumtext :medium_text t.longtext :long_text + t.binary :tiny_blob_2, size: :tiny + t.binary :medium_blob_2, size: :medium + t.binary :long_blob_2, size: :long + t.text :tiny_text_2, size: :tiny + t.text :medium_text_2, size: :medium + t.text :long_text_2, size: :long + t.index :var_binary end