Merge pull request rails#26655 from kamipo/fix_remove_expression_index

Fix `remove_index` to be able to remove expression indexes

Author: rafaelfranca

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

@@ -768,7 +768,7 @@ def index_name(table_name, options) #:nodoc:
             raise ArgumentError, "You must specify the index name"
           end
         else
-          index_name(table_name, column: options)
+          index_name(table_name, index_name_options(options))
         end
       end
 
@@ -1123,18 +1123,14 @@ def update_table_definition(table_name, base) #:nodoc:
       end
 
       def add_index_options(table_name, column_name, comment: nil, **options) # :nodoc:
-        if column_name.is_a?(String) && /\W/.match?(column_name)
-          column_names = column_name
-        else
-          column_names = Array(column_name)
-        end
+        column_names = index_column_names(column_name)
 
         options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type)
 
         index_type = options[:type].to_s if options.key?(:type)
         index_type ||= options[:unique] ? "UNIQUE" : ""
         index_name = options[:name].to_s if options.key?(:name)
-        index_name ||= index_name(table_name, index_name_options(column_names))
+        index_name ||= index_name(table_name, column_names)
 
         if options.key?(:algorithm)
           algorithm = index_algorithms.fetch(options[:algorithm]) {
@@ -1214,13 +1210,13 @@ def index_name_for_remove(table_name, options = {})
 
           if options.is_a?(Hash)
             checks << lambda { |i| i.name == options[:name].to_s } if options.key?(:name)
-            column_names = Array(options[:column]).map(&:to_s)
+            column_names = index_column_names(options[:column])
           else
-            column_names = Array(options).map(&:to_s)
+            column_names = index_column_names(options)
           end
 
-          if column_names.any?
-            checks << lambda { |i| i.columns.join("_and_") == column_names.join("_and_") }
+          if column_names.present?
+            checks << lambda { |i| index_name(table_name, i.columns) == index_name(table_name, column_names) }
           end
 
           raise ArgumentError, "No name or columns specified" if checks.none?
@@ -1267,8 +1263,16 @@ def create_alter_table(name)
           AlterTable.new create_table_definition(name)
         end
 
+        def index_column_names(column_names)
+          if column_names.is_a?(String) && /\W/.match?(column_names)
+            column_names
+          else
+            Array(column_names)
+          end
+        end
+
         def index_name_options(column_names)
-          if column_names.is_a?(String)
+          if column_names.is_a?(String) && /\W/.match?(column_names)
             column_names = column_names.scan(/\w+/).join("_")
           end
 

activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb

@@ -263,9 +263,12 @@ def test_expression_index
 
       def test_index_with_opclass
         with_example_table do
-          @connection.add_index "ex", "data varchar_pattern_ops", name: "with_opclass"
-          index = @connection.indexes("ex").find { |idx| idx.name == "with_opclass" }
+          @connection.add_index "ex", "data varchar_pattern_ops"
+          index = @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data_varchar_pattern_ops" }
           assert_equal "data varchar_pattern_ops", index.columns
+
+          @connection.remove_index "ex", "data varchar_pattern_ops"
+          assert_not @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data_varchar_pattern_ops" }
         end
       end