aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorYves Senn <yves.senn@gmail.com>2012-12-28 01:27:26 +0100
committerYves Senn <yves.senn@gmail.com>2013-02-20 17:50:10 +0100
commitcca43528d40589556401408eb3c315cde199347c (patch)
treecd2794639db357bbb140e2263eb233f88cb4ff47 /activerecord
parent7bae72c69f2352fd7e02f2d08adf873e4354aea0 (diff)
downloadrails-cca43528d40589556401408eb3c315cde199347c.tar.gz
rails-cca43528d40589556401408eb3c315cde199347c.tar.bz2
rails-cca43528d40589556401408eb3c315cde199347c.zip
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.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md8
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb9
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb8
-rw-r--r--activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb9
-rw-r--r--activerecord/test/cases/migration/index_test.rb28
-rw-r--r--activerecord/test/cases/migration/rename_column_test.rb11
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"