Skip to content

Commit

Permalink
Fix add_foreign_key when referencing same table via different columns
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Dec 5, 2024
1 parent d95670e commit fc016e4
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master (unreleased)

- Fix `add_foreign_key` when referencing same table via different columns
- Make `validate_not_null_constraint`, `remove_foreign_key` and `remove_check_constraint` idempotent

- Add helpers for validating constraints in background
Expand Down
15 changes: 7 additions & 8 deletions lib/online_migrations/schema_statements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -813,13 +813,13 @@ def index_name(table_name, options)
# @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_foreign_key
#
def add_foreign_key(from_table, to_table, **options)
# Do not consider validation for idempotency.
if foreign_key_exists?(from_table, to_table, **options.except(:validate))
message = +"Foreign key was not created because it already exists " \
"(this can be due to an aborted migration or similar): from_table: #{from_table}, to_table: #{to_table}"
message << ", #{options.inspect}" if options.any?
options = foreign_key_options(from_table, to_table, options)

Utils.say(message)
if foreign_key_exists?(from_table, to_table, **options.slice(:column, :primary_key))
Utils.say(<<~MSG.squish)
Foreign key was not created because it already exists (this can be due to an aborted migration or similar).
from_table: #{from_table}, to_table: #{to_table}, options: #{options.inspect}
MSG
else
super
end
Expand Down Expand Up @@ -859,8 +859,7 @@ def validate_foreign_key(from_table, to_table = nil, **options)
# @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-remove_foreign_key
#
def remove_foreign_key(from_table, to_table = nil, **options)
# Do not consider validation for idempotency.
if foreign_key_exists?(from_table, to_table, **options.except(:validate))
if foreign_key_exists?(from_table, to_table, **options.slice(:name, :to_table, :column))
super
else
Utils.say(<<~MSG.squish)
Expand Down
9 changes: 9 additions & 0 deletions test/schema_statements/foreign_keys_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ def setup
@connection.create_table(:milestones, force: true) do |t|
t.bigint :owner_id
t.bigint :project_id
t.bigint :parent_project_id
end
end

Expand All @@ -30,6 +31,14 @@ def test_add_foreign_key_is_idempotent
assert connection.foreign_key_exists?(:milestones, :projects)
end

def test_add_foreign_key_referencing_same_table_by_different_columns
connection.add_foreign_key :milestones, :projects, column: :parent_project_id
assert_equal 1, connection.foreign_keys(:milestones).size

connection.add_foreign_key :milestones, :projects
assert_equal 2, connection.foreign_keys(:milestones).size
end

def test_validate_non_existing_foreign_key
assert_raises_with_message(ArgumentError, "has no foreign key") do
connection.validate_foreign_key :milestones, :non_existing
Expand Down

0 comments on commit fc016e4

Please sign in to comment.