Merge pull request #14034 from hdabrows/drop-correct-index-when-reverting-migration

Drop the correct index after reverting a migration

4 files changed, 49 insertions, 4 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 1fd9003009..d8bb31a3a7 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,15 @@
+*   When inverting add_index use the index name if present instead of
+    the columns.
+
+    If there are two indices with matching columns and one of them is
+    explicitly named then reverting the migration adding the named one
+    would instead drop the unnamed one.
+
+    The inversion of add_index will now drop the index by its name if
+    it is present.
+
+    *Hubert Dąbrowski*
+
 *   Add flag to disable schema dump after migration.
 
     Add a config parameter on Active Record named `dump_schema_after_migration`
diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb
index 9139ad953c..c44d8c1665 100644
--- a/activerecord/lib/active_record/migration/command_recorder.rb
+++ b/activerecord/lib/active_record/migration/command_recorder.rb
@@ -140,7 +140,12 @@ module ActiveRecord
 
       def invert_add_index(args)
         table, columns, options = *args
-        [:remove_index, [table, (options || {}).merge(column: columns)]]
+        options ||= {}
+
+        index_name = options[:name]
+        options_hash = index_name ? { name: index_name } : { column: columns }
+
+        [:remove_index, [table, options_hash]]
       end
 
       def invert_remove_index(args)
diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb
index 428145d00b..debacf815c 100644
--- a/activerecord/test/cases/invertible_migration_test.rb
+++ b/activerecord/test/cases/invertible_migration_test.rb
@@ -106,6 +106,22 @@ module ActiveRecord
       end
     end
 
+    class RevertNamedIndexMigration1 < SilentMigration
+      def change
+        create_table("horses") do |t|
+          t.column :content, :string
+          t.column :remind_at, :datetime
+        end
+        add_index :horses, :content
+      end
+    end
+
+    class RevertNamedIndexMigration2 < SilentMigration
+      def change
+        add_index :horses, :content, name: "horses_index_named"
+      end
+    end
+
     def teardown
       %w[horses new_horses].each do |table|
         if ActiveRecord::Base.connection.table_exists?(table)
@@ -255,5 +271,17 @@ module ActiveRecord
       ActiveRecord::Base.table_name_prefix = ActiveRecord::Base.table_name_suffix = ''
     end
 
+    def test_migrate_revert_add_index_with_name
+      RevertNamedIndexMigration1.new.migrate(:up)
+      RevertNamedIndexMigration2.new.migrate(:up)
+      RevertNamedIndexMigration2.new.migrate(:down)
+
+      connection = ActiveRecord::Base.connection
+      assert connection.index_exists?(:horses, :content),
+             "index on content should exist"
+      assert !connection.index_exists?(:horses, :content, name: "horses_index_named"),
+             "horses_index_named index should not exist"
+    end
+
   end
 end
diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb
index 35b656ee43..a925cf4c05 100644
--- a/activerecord/test/cases/migration/command_recorder_test.rb
+++ b/activerecord/test/cases/migration/command_recorder_test.rb
@@ -174,13 +174,13 @@ module ActiveRecord
       end
 
       def test_invert_add_index
-        remove = @recorder.inverse_of :add_index, [:table, [:one, :two], options: true]
-        assert_equal [:remove_index, [:table, {column: [:one, :two], options: true}]], remove
+        remove = @recorder.inverse_of :add_index, [:table, [:one, :two]]
+        assert_equal [:remove_index, [:table, {column: [:one, :two]}]], remove
       end
 
       def test_invert_add_index_with_name
         remove = @recorder.inverse_of :add_index, [:table, [:one, :two], name: "new_index"]
-        assert_equal [:remove_index, [:table, {column: [:one, :two], name: "new_index"}]], remove
+        assert_equal [:remove_index, [:table, {name: "new_index"}]], remove
       end
 
       def test_invert_add_index_with_no_options