From 7138b467fe278f8e7c0a0b426f4595cc064b9cf2 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 4 Mar 2017 16:47:43 +0900 Subject: [PATCH] Don't share `options` with a reference type column Sharing `options` causes some unexpected behavior. If `limit: 2` is specified, this means that 2 bytes integer for a reference id column and 2 chars string for a reference type column. Another example, if `unsigned: true` is specified, this means that unsigned integer for a reference id column, but a invalid option for a reference type column. So `options` should not be shared with a reference type column. (cherry picked from commit 465357aecfcfd5ae0b1f95b8a848e38ac868999d) --- activerecord/CHANGELOG.md | 10 ++++++++++ .../connection_adapters/abstract/schema_definitions.rb | 10 +++------- .../test/cases/migration/references_statements_test.rb | 7 +++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a39e03a1e8..510f19ba0e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,13 @@ +* Don't share options between reference id and type columns + + When using a polymorphic reference column in a migration, sharing options + between the two columns doesn't make sense since they are different types. + The `reference_id` column is usually an integer and the `reference_type` + column a string so options like `unsigned: true` will result in an invalid + table definition. + + *Ryuta Kamizono* + * Fix regression of #1969 with SELECT aliases in HAVING clause. *Eugene Kenny* 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 3fd44019c2..61ca7dafb4 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -107,16 +107,12 @@ module ActiveRecord private - def as_options(value, default = {}) - if value.is_a?(Hash) - value - else - default - end + def as_options(value) + value.is_a?(Hash) ? value : {} end def polymorphic_options - as_options(polymorphic, options) + as_options(polymorphic) end def index_options diff --git a/activerecord/test/cases/migration/references_statements_test.rb b/activerecord/test/cases/migration/references_statements_test.rb index 70c64f3e71..08f0551d6f 100644 --- a/activerecord/test/cases/migration/references_statements_test.rb +++ b/activerecord/test/cases/migration/references_statements_test.rb @@ -50,6 +50,13 @@ module ActiveRecord assert column_exists?(table_name, :taggable_type, :string, default: 'Photo') end + def test_does_not_share_options_with_reference_type_column + add_reference table_name, :taggable, type: :integer, limit: 2, polymorphic: true + assert column_exists?(table_name, :taggable_id, :integer, limit: 2) + assert column_exists?(table_name, :taggable_type, :string) + assert_not column_exists?(table_name, :taggable_type, :string, limit: 2) + end + def test_creates_named_index add_reference table_name, :tag, index: { name: 'index_taggings_on_tag_id' } assert index_exists?(table_name, :tag_id, name: 'index_taggings_on_tag_id')