Merge pull request rails#53863 from fatkodima/fix-if-exists-for-fkeys

Fix `if_exists`/`if_not_exists` for foreign keys addition and removal

Author: byroot

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

@@ -1172,9 +1172,10 @@ def foreign_keys(table_name)
       #   +:deferred+ or +:immediate+ to specify the default behavior. Defaults to +false+.
       def add_foreign_key(from_table, to_table, **options)
         return unless use_foreign_keys?
-        return if options[:if_not_exists] == true && foreign_key_exists?(from_table, to_table, **options.slice(:column))
 
         options = foreign_key_options(from_table, to_table, options)
+        return if options[:if_not_exists] == true && foreign_key_exists?(from_table, to_table, **options.slice(:column, :primary_key))
+
         at = create_alter_table from_table
         at.add_foreign_key to_table, options
 
@@ -1213,7 +1214,7 @@ def add_foreign_key(from_table, to_table, **options)
       #   The name of the table that contains the referenced primary key.
       def remove_foreign_key(from_table, to_table = nil, **options)
         return unless use_foreign_keys?
-        return if options.delete(:if_exists) == true && !foreign_key_exists?(from_table, to_table)
+        return if options.delete(:if_exists) == true && !foreign_key_exists?(from_table, to_table, **options.slice(:column))
 
         fk_name_to_delete = foreign_key_for!(from_table, to_table: to_table, **options).name
 

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

@@ -63,7 +63,7 @@ def add_foreign_key(from_table, to_table, **options)
         end
 
         def remove_foreign_key(from_table, to_table = nil, **options)
-          return if options.delete(:if_exists) == true && !foreign_key_exists?(from_table, to_table)
+          return if options.delete(:if_exists) == true && !foreign_key_exists?(from_table, to_table, **options.slice(:column))
 
           to_table ||= options[:to_table]
           options = options.except(:name, :to_table, :validate)

activerecord/test/cases/migration/foreign_key_test.rb

@@ -235,15 +235,31 @@ def test_add_foreign_key_with_column
         end
 
         def test_add_foreign_key_with_if_not_exists_to_already_referenced_table
-          @connection.add_foreign_key :astronauts, :rockets
-          @connection.add_foreign_key :astronauts, :rockets, column: "favorite_rocket_id", if_not_exists: true
+          @connection.add_foreign_key :astronauts, :rockets, column: "favorite_rocket_id"
+          @connection.add_foreign_key :astronauts, :rockets, if_not_exists: true
 
           foreign_keys = @connection.foreign_keys("astronauts")
           assert_equal 2, foreign_keys.size
           assert foreign_keys.all? { |fk| fk.to_table == "rockets" }
           assert_equal ["favorite_rocket_id", "rocket_id"], foreign_keys.map(&:column).sort
         end
 
+        def test_add_foreign_key_with_if_not_exists_considers_primary_key_option
+          @connection.add_column :rockets, :id_for_type_change, :bigint
+
+          # Is needed to be able to reference by foreign key
+          @connection.add_index :rockets, :id_for_type_change, unique: true
+
+          @connection.add_foreign_key :astronauts, :rockets
+          @connection.add_foreign_key(:astronauts, :rockets, primary_key: :id_for_type_change,
+            name: "custom_pk",  if_not_exists: true)
+
+          foreign_keys = @connection.foreign_keys("astronauts")
+          assert_equal 2, foreign_keys.size
+          assert foreign_keys.all? { |fk| fk.to_table == "rockets" }
+          assert_equal ["id", "id_for_type_change"], foreign_keys.map(&:primary_key).sort
+        end
+
         def test_add_foreign_key_with_non_standard_primary_key
           @connection.create_table :space_shuttles, id: false, force: true do |t|
             t.bigint :pk, primary_key: true
@@ -424,6 +440,15 @@ def test_remove_foreign_key_by_name
           assert_equal [], @connection.foreign_keys("astronauts")
         end
 
+        def test_remove_foreign_key_if_exists_and_custom_column
+          @connection.add_column :astronauts, :myrocket_id, :bigint
+          @connection.add_foreign_key :astronauts, :rockets
+          assert_equal 1, @connection.foreign_keys("astronauts").size
+
+          @connection.remove_foreign_key :astronauts, :rockets, column: :myrocket_id, if_exists: true
+          assert_equal 1, @connection.foreign_keys("astronauts").size
+        end
+
         def test_remove_foreign_non_existing_foreign_key_raises
           e = assert_raises ArgumentError do
             @connection.remove_foreign_key :astronauts, :rockets