also rename indexes when a table or column is renamed

When a table or a column is renamed related indexes kept their name. This will lead to confusing names. This patch renames related indexes when a column or a table is renamed. Only indexes with names generated by rails will be renamed. Indexes with custom names will not be renamed.

7 files changed, 73 insertions, 8 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 1358c546e9..4ff791a29f 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,10 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Rename related indexes on `rename_table` and `rename_column`. This
+    does not affect indexes with custom names.
+
+    *Yves Senn*
+
 *   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
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 2c0a18fd01..b737268f61 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -696,6 +696,28 @@ module ActiveRecord
           column_names.map {|column_name| quote_column_name(column_name) }
         end
 
+        def rename_table_indexes(table_name, new_name)
+          indexes(new_name).each do |index|
+            generated_index_name = index_name(table_name, column: index.columns)
+            if generated_index_name == index.name
+              rename_index new_name, generated_index_name, index_name(new_name, column: index.columns)
+            end
+          end
+        end
+
+        def rename_column_indexes(table_name, column_name, new_column_name)
+          column_name, new_column_name = column_name.to_s, new_column_name.to_s
+          indexes(table_name).each do |index|
+            next unless index.columns.include?(new_column_name)
+            old_columns = index.columns.dup
+            old_columns[old_columns.index(new_column_name)] = column_name
+            generated_index_name = index_name(table_name, column: old_columns)
+            if generated_index_name == index.name
+              rename_index table_name, generated_index_name, index_name(table_name, column: index.columns)
+            end
+          end
+        end
+
       private
       def table_definition
         TableDefinition.new(self)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index e07dbc7da9..0871454cfe 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -468,6 +468,7 @@ module ActiveRecord
       #   rename_table('octopuses', 'octopi')
       def rename_table(table_name, new_name)
         execute "RENAME TABLE #{quote_table_name(table_name)} TO #{quote_table_name(new_name)}"
+        rename_table_indexes(table_name, new_name)
       end
 
       def add_column(table_name, column_name, type, options = {})
@@ -495,6 +496,7 @@ module ActiveRecord
 
       def rename_column(table_name, column_name, new_column_name) #:nodoc:
         execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_sql(table_name, column_name, new_column_name)}")
+        rename_column_indexes(table_name, column_name, new_column_name)
       end
 
       # Maps logical Rails types to MySQL-specific data types.
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index 73ca2c8e61..3bc61c5e0c 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -321,14 +321,16 @@ module ActiveRecord
         #
         # Example:
         #   rename_table('octopuses', 'octopi')
-        def rename_table(name, new_name)
+        def rename_table(table_name, new_name)
           clear_cache!
-          execute "ALTER TABLE #{quote_table_name(name)} RENAME TO #{quote_table_name(new_name)}"
+          execute "ALTER TABLE #{quote_table_name(table_name)} RENAME TO #{quote_table_name(new_name)}"
           pk, seq = pk_and_sequence_for(new_name)
-          if seq == "#{name}_#{pk}_seq"
+          if seq == "#{table_name}_#{pk}_seq"
             new_seq = "#{new_name}_#{pk}_seq"
             execute "ALTER TABLE #{quote_table_name(seq)} RENAME TO #{quote_table_name(new_seq)}"
           end
+
+          rename_table_indexes(table_name, new_name)
         end
 
         # Adds a new column to the named table.
@@ -370,6 +372,7 @@ module ActiveRecord
         def rename_column(table_name, column_name, new_column_name)
           clear_cache!
           execute "ALTER TABLE #{quote_table_name(table_name)} RENAME COLUMN #{quote_column_name(column_name)} TO #{quote_column_name(new_column_name)}"
+          rename_column_indexes(table_name, column_name, new_column_name)
         end
 
         def remove_index!(table_name, index_name) #:nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 105ba69028..6ede5f3390 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -435,8 +435,9 @@ module ActiveRecord
       #
       # Example:
       #   rename_table('octopuses', 'octopi')
-      def rename_table(name, new_name)
-        exec_query "ALTER TABLE #{quote_table_name(name)} RENAME TO #{quote_table_name(new_name)}"
+      def rename_table(table_name, new_name)
+        exec_query "ALTER TABLE #{quote_table_name(table_name)} RENAME TO #{quote_table_name(new_name)}"
+        rename_table_indexes(table_name, new_name)
       end
 
       # See: http://www.sqlite.org/lang_altertable.html
@@ -495,6 +496,7 @@ module ActiveRecord
           raise ActiveRecord::ActiveRecordError, "Missing column #{table_name}.#{column_name}"
         end
         alter_table(table_name, :rename => {column_name.to_s => new_column_name.to_s})
+        rename_column_indexes(table_name, column_name, new_column_name)
       end
 
       protected
diff --git a/activerecord/test/cases/migration/rename_column_test.rb b/activerecord/test/cases/migration/rename_column_test.rb
index b116753d04..88bea2211d 100644
--- a/activerecord/test/cases/migration/rename_column_test.rb
+++ b/activerecord/test/cases/migration/rename_column_test.rb
@@ -86,8 +86,29 @@ module ActiveRecord
 
         assert_equal 1, connection.indexes('test_models').size
         rename_column "test_models", "hat_name", "name"
-        # FIXME: should we rename the index if it's name was autogenerated by rails?
-        assert_equal ['index_test_models_on_hat_name'], connection.indexes('test_models').map(&:name)
+
+        assert_equal ['index_test_models_on_name'], connection.indexes('test_models').map(&:name)
+      end
+
+      def test_rename_column_with_multi_column_index
+        add_column "test_models", :hat_size, :integer
+        add_column "test_models", :hat_style, :string, limit: 100
+        add_index "test_models", ["hat_style", "hat_size"], unique: true
+
+        rename_column "test_models", "hat_size", 'size'
+        assert_equal ['index_test_models_on_hat_style_and_size'], connection.indexes('test_models').map(&:name)
+
+        rename_column "test_models", "hat_style", 'style'
+        assert_equal ['index_test_models_on_style_and_size'], connection.indexes('test_models').map(&:name)
+      end
+
+      def test_rename_column_does_not_rename_custom_named_index
+        add_column "test_models", :hat_name, :string
+        add_index :test_models, :hat_name, :name => 'idx_hat_name'
+
+        assert_equal 1, connection.indexes('test_models').size
+        rename_column "test_models", "hat_name", "name"
+        assert_equal ['idx_hat_name'], connection.indexes('test_models').map(&:name)
       end
 
       def test_remove_column_with_index
diff --git a/activerecord/test/cases/migration/rename_table_test.rb b/activerecord/test/cases/migration/rename_table_test.rb
index 21901bec3c..22dbd7c38b 100644
--- a/activerecord/test/cases/migration/rename_table_test.rb
+++ b/activerecord/test/cases/migration/rename_table_test.rb
@@ -63,7 +63,17 @@ module ActiveRecord
         connection.enable_identity_insert("octopi", false) if current_adapter?(:SybaseAdapter)
 
         assert_equal 'http://www.foreverflying.com/octopus-black7.jpg', connection.select_value("SELECT url FROM octopi WHERE id=1")
-        assert connection.indexes(:octopi).first.columns.include?("url")
+        index = connection.indexes(:octopi).first
+        assert index.columns.include?("url")
+        assert_equal 'index_octopi_on_url', index.name
+      end
+
+      def test_rename_table_does_not_rename_custom_named_index
+        add_index :test_models, :url, name: 'special_url_idx'
+
+        rename_table :test_models, :octopi
+
+        assert_equal ['special_url_idx'], connection.indexes(:octopi).map(&:name)
       end
 
       def test_rename_table_for_postgresql_should_also_rename_default_sequence