Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

reserve index name chars for internal rails operations

Some adapter (SQLite3) need to perform renaming operations to support
the rails DDL. These rename prefixes operate with prefixes. When an
index name already uses up the full space provieded by
`index_name_length` these internal operations will fail. This patch
introduces `allowed_index_name_length` which respects the amount of
characters used for internal operations. It will always be <=
`index_name_length` and every adapter can define how many characters
need to be reserved.
  • Loading branch information...
commit cca43528d40589556401408eb3c315cde199347c 1 parent 7bae72c
@senny senny authored
View
8 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
View
9 activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb
@@ -17,6 +17,15 @@ def table_name_length
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
View
8 activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -647,10 +647,11 @@ def add_index_options(table_name, column_name, options = {})
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 @@ def add_index_options(table_name, column_name, options = {})
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"
View
9 activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -187,6 +187,13 @@ def supports_index_sort_order?
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 @@ def copy_table_indexes(from, to, rename = {}) #:nodoc:
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
View
28 activerecord/test/cases/migration/index_test.rb
@@ -55,19 +55,31 @@ def test_remove_nonexistent_index
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 @@ def test_add_partial_index
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
View
11 activerecord/test/cases/migration/rename_column_test.rb
@@ -197,6 +197,17 @@ def test_change_column_with_custom_index_name
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"
Please sign in to comment.
Something went wrong with that request. Please try again.