Add if_exists option to remove_index

This PR allows for passing `if_exists` options to the `remove_index`
method so that we can ignore already removed indexes. This work follows
column `if/if_not_exists` from rails#38352 and `:if_not_exists` on `add_index`
from rails#38555.

We've found this useful at GitHub, there are migrations where we don't
want to raise if an index was already removed. This will allow us to
remove a monkey patch on `remove_index`.

I considered raising after the `index_name_for_remove` method is called
but that method will raise if the index doesn't exist before we get to
execute. I have a commit that refactors this but after much
consideration this change is cleaner and more straightforward than other
ways of implementing this.

This change also adds a little extra validation to the `add_index` test.
Fix `nodoc` on edited methods.

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Add support for `if_exists` option for removing an index.
+
+    The `remove_index` method can take an `if_exists` option. If this is set to true an error won't be raised if the index doesn't exist.
+
+    *Eileen M. Uchitelle*
+
 *   Remove ibm_db, informix, mssql, oracle, and oracle12 Arel visitors which are not used in the code base.
 
     *Ryuta Kamizono*

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

@@ -845,6 +845,11 @@ def add_index(table_name, column_name, options = {})
       #
       #   remove_index :accounts, :branch_id, name: :by_branch_party
       #
+      # Checks if the index exists before trying to remove it. Will silently ignore indexes that
+      # don't exist.
+      #
+      #   remove_index :accounts, if_exists: true
+      #
       # Removes the index named +by_branch_party+ in the +accounts+ table +concurrently+.
       #
       #   remove_index :accounts, name: :by_branch_party, algorithm: :concurrently
@@ -855,7 +860,10 @@ def add_index(table_name, column_name, options = {})
       #
       # For more information see the {"Transactional Migrations" section}[rdoc-ref:Migration].
       def remove_index(table_name, column_name = nil, options = {})
+        return if options[:if_exists] && !index_exists?(table_name, column_name, options)
+
         index_name = index_name_for_remove(table_name, column_name, options)
+
         execute "DROP INDEX #{quote_column_name(index_name)} ON #{quote_table_name(table_name)}"
       end
 

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

@@ -448,7 +448,7 @@ def add_index(table_name, column_name, options = {}) #:nodoc:
           end
         end
 
-        def remove_index(table_name, column_name = nil, options = {}) #:nodoc:
+        def remove_index(table_name, column_name = nil, options = {}) # :nodoc:
           table = Utils.extract_schema_qualified_name(table_name.to_s)
 
           if column_name.is_a?(Hash)
@@ -467,13 +467,17 @@ def remove_index(table_name, column_name = nil, options = {}) #:nodoc:
             end
           end
 
+          return if options[:if_exists] && !index_exists?(table_name, column_name, options)
+
           index_to_remove = PostgreSQL::Name.new(table.schema, index_name_for_remove(table.to_s, column_name, options))
+
           algorithm =
             if options.key?(:algorithm)
               index_algorithms.fetch(options[:algorithm]) do
                 raise ArgumentError.new("Algorithm must be one of the following: #{index_algorithms.keys.map(&:inspect).join(', ')}")
               end
             end
+
           execute "DROP INDEX #{algorithm} #{quote_table_name(index_to_remove)}"
         end
 

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -226,8 +226,11 @@ def primary_keys(table_name) # :nodoc:
         pks.sort_by { |f| f["pk"] }.map { |f| f["name"] }
       end
 
-      def remove_index(table_name, column_name, options = {}) #:nodoc:
+      def remove_index(table_name, column_name, options = {}) # :nodoc:
+        return if options[:if_exists] && !index_exists?(table_name, column_name, options)
+
         index_name = index_name_for_remove(table_name, column_name, options)
+
         exec_query "DROP INDEX #{quote_column_name(index_name)}"
       end
 

activerecord/test/cases/migration/index_test.rb

@@ -72,12 +72,28 @@ def test_add_index_does_not_accept_too_long_index_names
         connection.add_index(table_name, "foo", name: good_index_name)
       end
 
-      def test_add_index_which_already_exists_does_not_raise_error
+      def test_add_index_which_already_exists_does_not_raise_error_with_option
         connection.add_index(table_name, "foo", if_not_exists: true)
 
         assert_nothing_raised do
           connection.add_index(table_name, "foo", if_not_exists: true)
         end
+
+        assert connection.index_name_exists?(table_name, "index_testings_on_foo")
+      end
+
+      def test_remove_index_which_does_not_exist_doesnt_raise_with_option
+        connection.add_index(table_name, "foo")
+
+        connection.remove_index(table_name, "foo")
+
+        assert_raises ArgumentError do
+          connection.remove_index(table_name, "foo")
+        end
+
+        assert_nothing_raised do
+          connection.remove_index(table_name, "foo", if_exists: true)
+        end
       end
 
       def test_internal_index_with_name_matching_database_limit