Skip to content

Commit b6226c3

Browse files
committed
If an index can't be found by column, use the index name.
schema_statements uses the column name by default to construct the index name, and then raises an exception if it doesn't exist, even if the name option is specified, which causes rails#8858. this commit makes index_name_for_remove fall back to constructing the index name to remove based on the name option.
1 parent b670433 commit b6226c3

File tree

3 files changed

+32
-0
lines changed

3 files changed

+32
-0
lines changed

activerecord/CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
## Rails 4.0.0 (unreleased) ##
22

3+
* When `:name` option is provided to `remove_index`, use it if there is no index by the conventional name.
4+
5+
For example, previously if an index was removed like so
6+
`remove_index :values, column: :value, name: 'a_different_name'`
7+
the generated SQL would not contain the specified index name,
8+
and hence the migration would fail.
9+
Fixes #8858.
10+
11+
*Ezekiel Smithburg*
12+
313
* Expand `#cache_key` to consult all relevant updated timestamps.
414

515
Previously only `updated_at` column was checked, now it will

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,12 @@ def index_name_for_remove(table_name, options = {})
687687
index_name = index_name(table_name, options)
688688

689689
unless index_name_exists?(table_name, index_name, true)
690+
if options.has_key? :name
691+
options_without_column = options.dup
692+
options_without_column.delete :column
693+
index_name_without_column = index_name(table_name, options_without_column)
694+
return index_name_without_column if index_name_exists?(table_name, index_name_without_column, false)
695+
end
690696
raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' does not exist"
691697
end
692698

activerecord/test/cases/migration_test.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,22 @@ def test_drop_index_from_table_named_values
462462
end
463463
end
464464

465+
class ExplicitlyNamedIndexMigrationTest < ActiveRecord::TestCase
466+
def test_drop_index_by_name
467+
connection = Person.connection
468+
connection.create_table :values, force: true do |t|
469+
t.integer :value
470+
end
471+
472+
assert_nothing_raised ArgumentError do
473+
connection.add_index :values, :value, name: 'a_different_name'
474+
connection.remove_index :values, column: :value, name: 'a_different_name'
475+
end
476+
477+
connection.drop_table :values rescue nil
478+
end
479+
end
480+
465481
if ActiveRecord::Base.connection.supports_bulk_alter?
466482
class BulkAlterTableMigrationsTest < ActiveRecord::TestCase
467483
def setup

0 commit comments

Comments
 (0)