From 33b712bc8cda3f8f91c4334f72a8b6c5a4849307 Mon Sep 17 00:00:00 2001
From: Suraj Mishra <surmishra@paypal.com>
Date: Wed, 26 Jun 2024 03:04:00 +0000
Subject: [PATCH] fixing bug to support both string and symbol for constraint
 name

---
 lib/pg_ha_migrations/safe_statements.rb |  6 +++++
 spec/safe_statements_spec.rb            | 36 +++++++++----------------
 2 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/lib/pg_ha_migrations/safe_statements.rb b/lib/pg_ha_migrations/safe_statements.rb
index 0a6ef04..0c5a057 100644
--- a/lib/pg_ha_migrations/safe_statements.rb
+++ b/lib/pg_ha_migrations/safe_statements.rb
@@ -598,6 +598,12 @@ def safely_acquire_lock_for_table(table, mode: :access_exclusive, &block)
     Thread.current[__method__] = nil unless nested_target_table
   end
 
+  # since rails versions < 7.1 has bug which does not handle symbol for
+  # constraint name, we are converting name to string explicitly to solve that.
+  def unsafe_remove_check_constraint(table, name:, **options)
+    super(table, name: name.to_s, **options)
+  end
+
   def adjust_lock_timeout(timeout_seconds = PgHaMigrations::LOCK_TIMEOUT_SECONDS, &block)
     _check_postgres_adapter!
     original_timeout = ActiveRecord::Base.value_from_sql("SHOW lock_timeout").sub(/s\Z/, '').to_i * 1000
diff --git a/spec/safe_statements_spec.rb b/spec/safe_statements_spec.rb
index 9a27831..ecffa0f 100644
--- a/spec/safe_statements_spec.rb
+++ b/spec/safe_statements_spec.rb
@@ -1280,32 +1280,20 @@ def up
         end
 
         describe "unsafe_remove_check_constraint" do
-          it "throws error when symbol is used for contraint name" do
-            migration = Class.new(migration_klass) do
-              define_method(:up) do
-                unsafe_create_table :foos
-                unsafe_add_column :foos, :bar, :text
-                unsafe_add_check_constraint :foos, "bar IS NOT NULL", :name => :constraint_foo_bar_is_not_null
-                unsafe_remove_check_constraint :foos, name: :constraint_foo_bar_is_not_null
-              end
-            end
-            expect do
-              migration.suppress_messages { migration.migrate(:up) }
-            end.to raise_error(ArgumentError, /Table 'foos' has no check constraint for {:name=>:constraint_foo_bar_is_not_null}/)
-          end
-
-          it "doesn't throw error when string is used for constraint name" do
-            migration = Class.new(migration_klass) do
-              define_method(:up) do
-                unsafe_create_table :foos
-                unsafe_add_column :foos, :bar, :text
-                unsafe_add_check_constraint :foos, "bar IS NOT NULL", :name => :constraint_foo_bar_is_not_null
-                unsafe_remove_check_constraint :foos, name: "constraint_foo_bar_is_not_null"
+          ["constraint_foo_bar_is_not_null", :constraint_foo_bar_is_not_null].each do |constraint_name|
+            it "does not throw an error when #{constraint_name.is_a?(Symbol) ? "symbol" : "string"} is used for contraint name" do
+              migration = Class.new(migration_klass) do
+                define_method(:up) do
+                  unsafe_create_table :foos
+                  unsafe_add_column :foos, :bar, :text
+                  unsafe_add_check_constraint :foos, "bar IS NOT NULL", :name => :constraint_foo_bar_is_not_null
+                  unsafe_remove_check_constraint :foos, name: constraint_name
+                end
               end
+              expect do
+                migration.suppress_messages { migration.migrate(:up) }
+              end.not_to raise_error
             end
-            expect do
-              migration.suppress_messages { migration.migrate(:up) }
-            end.not_to raise_error
           end
         end