Merge pull request #10437 from neerajdotname/10419

raise IrreversibleMigration if no column given

activerecord/CHANGELOG.md

+*   While removing index if column option is missing then raise IrreversibleMigration exception.
+
+    Following code should raise `IrreversibleMigration`. But the code was
+    failing since options is an array and not a hash.
+
+        def change
+          change_table :users do |t|
+            t.remove_index [:name, :email]
+          end
+        end
+
+    Fix was to check if the options is a Hash before operating on it.
+
+    Fixes #10419.
+
+    *Neeraj Singh*
+
 *   Do not overwrite manually built records during one-to-one nested attribute assignment
 
     For one-to-one nested associations, if you build the new (in-memory)

activerecord/lib/active_record/migration/command_recorder.rb

@@ -144,7 +144,10 @@ def invert_add_index(args)
 
       def invert_remove_index(args)
         table, options = *args
-        raise ActiveRecord::IrreversibleMigration, "remove_index is only reversible if given a :column option." unless options && options[:column]
+
+        unless options && options.is_a?(Hash) && options[:column]
+          raise ActiveRecord::IrreversibleMigration, "remove_index is only reversible if given a :column option."
+        end
 
         options = options.dup
         [:add_index, [table, options.delete(:column), options]]

activerecord/test/cases/invertible_migration_test.rb

@@ -58,6 +58,24 @@ def change
       end
     end
 
+    class RemoveIndexMigration1 < SilentMigration
+      def self.up
+        create_table("horses") do |t|
+          t.column :name, :text
+          t.column :color, :text
+          t.index [:name, :color]
+        end
+      end
+    end
+
+    class RemoveIndexMigration2 < SilentMigration
+      def change
+        change_table("horses") do |t|
+          t.remove_index [:name, :color]
+        end
+      end
+    end
+
     class LegacyMigration < ActiveRecord::Migration
       def self.up
         create_table("horses") do |t|
@@ -104,6 +122,16 @@ def test_no_reverse
       end
     end
 
+    def test_exception_on_removing_index_without_column_option
+      RemoveIndexMigration1.new.migrate(:up)
+      migration = RemoveIndexMigration2.new
+      migration.migrate(:up)
+
+      assert_raises(IrreversibleMigration) do
+        migration.migrate(:down)
+      end
+    end
+
     def test_migrate_up
       migration = InvertibleMigration.new
       migration.migrate(:up)