Improved handling of invalid indexes with add_index, safe_by_default,…

… and Active Record < 7.1 - #241

Co-authored-by: Diego Guerra <diego.guerra@toptal.com>

CHANGELOG.md

@@ -3,7 +3,7 @@
 - Added `skip_databases` option
 - Added warning for unsupported adapters
 - Improved output for `db:forward`, `db:rollback`, `db:migrate:up`, and `db:migrate:down`
-- Improved handling of invalid indexes with `add_index`, `safe_by_default`, and Active Record 7.1+
+- Improved handling of invalid indexes with `add_index` and `safe_by_default`
 
 ## 2.0.2 (2024-10-30)
 

lib/strong_migrations/adapters/postgresql_adapter.rb

@@ -142,6 +142,21 @@ def constraints(table_name)
         select_all(query.squish).to_a
       end
 
+      # TODO use indexes method when Active Record < 7.1 is no longer supported
+      def index_invalid?(table_name, index_name)
+        query = <<~SQL
+          SELECT
+            indisvalid
+          FROM
+            pg_index
+          WHERE
+            indrelid = to_regclass(#{connection.quote(connection.quote_table_name(table_name))}) AND
+            indexrelid = to_regclass(#{connection.quote(connection.quote_table_name(index_name))}) AND
+            indisvalid = false
+        SQL
+        select_all(query.squish).any?
+      end
+
       def writes_blocked?
         query = <<~SQL
           SELECT

lib/strong_migrations/safe_methods.rb

@@ -6,13 +6,11 @@ def safe_by_default_method?(method)
 
     def safe_add_index(*args, **options)
       disable_transaction
-      if ActiveRecord::VERSION::STRING.to_f >= 7.1
-        table, columns = args
-        index_name = options.fetch(:name, @migration.connection.index_name(table, columns))
-        if @migration.connection.indexes(table).any? { |i| i.name == index_name && !i.valid }
-          # TODO pass index schema for extra safety?
-          safe_remove_index(table, name: index_name)
-        end
+      table, columns = args
+      index_name = options.fetch(:name, @migration.connection.index_name(table, columns))
+      if adapter.index_invalid?(table, index_name)
+        # TODO pass index schema for extra safety?
+        safe_remove_index(table, name: index_name)
       end
       @migration.add_index(*args, **options.merge(algorithm: :concurrently))
     end

test/safe_by_default_test.rb

@@ -14,7 +14,7 @@ def test_add_index
   end
 
   def test_add_index_invalid
-    skip unless postgresql? && ActiveRecord::VERSION::STRING.to_f >= 7.1
+    skip unless postgresql?
 
     with_locked_table("users") do
       assert_raises(ActiveRecord::LockWaitTimeout) do