diff options
Diffstat (limited to 'activerecord')
6 files changed, 64 insertions, 9 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6098c2c425..f45b9219c7 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,13 @@ ## Rails 4.0.0 (unreleased) ## +* Prevent the creation of indices with too long names, which cause + internal operations to fail (sqlite3 adapter only). The method + `allowed_index_name_length` defines the length limit enforced by + rails. It's value defaults to `index_name_length` but can vary per adapter. + Fix #8264. + + *Yves Senn* + * Fixing issue #776. Memory bloat in transactions is handled by having the transaction hold only diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb index 30ccb8f0a4..2859fb31e8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb @@ -17,6 +17,15 @@ module ActiveRecord 64 end + # Returns the maximum allowed length for an index name. This + # limit is enforced by rails and Is less than or equal to + # <tt>index_name_length</tt>. The gap between + # <tt>index_name_length</tt> is to allow internal rails + # opreations to use prefixes in temporary opreations. + def allowed_index_name_length + index_name_length + end + # Returns the maximum length of an index name. def index_name_length 64 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 cdc8433185..2c0a18fd01 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -647,10 +647,11 @@ module ActiveRecord index_name = index_name(table_name, column: column_names) if Hash === options # legacy support, since this param was a string - options.assert_valid_keys(:unique, :order, :name, :where, :length) + options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal) index_type = options[:unique] ? "UNIQUE" : "" index_name = options[:name].to_s if options.key?(:name) + max_index_length = options.fetch(:internal, false) ? index_name_length : allowed_index_name_length if supports_partial_index? index_options = options[:where] ? " WHERE #{options[:where]}" : "" @@ -665,10 +666,11 @@ module ActiveRecord end index_type = options + max_index_length = allowed_index_name_length end - if index_name.length > index_name_length - raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' is too long; the limit is #{index_name_length} characters" + if index_name.length > max_index_length + raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' is too long; the limit is #{max_index_length} characters" end if index_name_exists?(table_name, index_name, false) raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' already exists" diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 91444950be..1b93af5033 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -187,6 +187,13 @@ module ActiveRecord true end + # Returns 51. SQLite supports index names up to 64 + # characters. The rest is used by rails internally to perform + # temporary rename operations + def allowed_index_name_length + index_name_length - 13 + end + def native_database_types #:nodoc: { :primary_key => default_primary_key_type, @@ -559,7 +566,7 @@ module ActiveRecord unless columns.empty? # index name can't be the same - opts = { name: name.gsub(/(^|_)(#{from})_/, "\\1#{to}_") } + opts = { name: name.gsub(/(^|_)(#{from})_/, "\\1#{to}_"), internal: true } opts[:unique] = true if index.unique add_index(to, columns, opts) end diff --git a/activerecord/test/cases/migration/index_test.rb b/activerecord/test/cases/migration/index_test.rb index a41f2c10f0..5df9a5342d 100644 --- a/activerecord/test/cases/migration/index_test.rb +++ b/activerecord/test/cases/migration/index_test.rb @@ -55,19 +55,31 @@ module ActiveRecord assert_raise(ArgumentError) { connection.remove_index(table_name, "no_such_index") } end - def test_add_index_name_length_limit - good_index_name = 'x' * connection.index_name_length + def test_add_index_works_with_long_index_names + connection.add_index(table_name, "foo", name: good_index_name) + + assert connection.index_name_exists?(table_name, good_index_name, false) + connection.remove_index(table_name, name: good_index_name) + end + + def test_add_index_does_not_accept_too_long_index_names too_long_index_name = good_index_name + 'x' - assert_raises(ArgumentError) { - connection.add_index(table_name, "foo", :name => too_long_index_name) + e = assert_raises(ArgumentError) { + connection.add_index(table_name, "foo", name: too_long_index_name) } + assert_match /too long; the limit is #{connection.allowed_index_name_length} characters/, e.message assert_not connection.index_name_exists?(table_name, too_long_index_name, false) connection.add_index(table_name, "foo", :name => good_index_name) + end + + def test_internal_index_with_name_matching_database_limit + good_index_name = 'x' * connection.index_name_length + connection.add_index(table_name, "foo", name: good_index_name, internal: true) assert connection.index_name_exists?(table_name, good_index_name, false) - connection.remove_index(table_name, :name => good_index_name) + connection.remove_index(table_name, name: good_index_name) end def test_index_symbol_names @@ -196,6 +208,12 @@ module ActiveRecord connection.remove_index("testings", "last_name") assert !connection.index_exists?("testings", "last_name") end + + private + def good_index_name + 'x' * connection.allowed_index_name_length + end + end end end diff --git a/activerecord/test/cases/migration/rename_column_test.rb b/activerecord/test/cases/migration/rename_column_test.rb index 8f6918d06a..b116753d04 100644 --- a/activerecord/test/cases/migration/rename_column_test.rb +++ b/activerecord/test/cases/migration/rename_column_test.rb @@ -197,6 +197,17 @@ module ActiveRecord assert_equal ['test_models_categories_idx'], connection.indexes('test_models').map(&:name) end + def test_change_column_with_long_index_name + table_name_prefix = 'test_models_' + long_index_name = table_name_prefix + ('x' * (connection.allowed_index_name_length - table_name_prefix.length)) + add_column "test_models", "category", :string + add_index :test_models, :category, name: long_index_name + + change_column "test_models", "category", :string, null: false, default: 'article' + + assert_equal [long_index_name], connection.indexes('test_models').map(&:name) + end + def test_change_column_default add_column "test_models", "first_name", :string connection.change_column_default "test_models", "first_name", "Tester" |