Skip to content

Commit

Permalink
Merge pull request rails#49738 from fatkodima/avoid-unneeded-queries-…
Browse files Browse the repository at this point in the history
…in-delete

Ignore invalid PRIMARY KEY values in `ActiveRecord::Persistence.delete` method
  • Loading branch information
byroot authored Oct 23, 2023
2 parents 7937092 + 39781d2 commit 55bf2df
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 0 deletions.
2 changes: 2 additions & 0 deletions activerecord/lib/active_record/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,8 @@ def destroy(id)
# # Delete multiple rows
# Todo.delete([2,3,4])
def delete(id_or_array)
return 0 if id_or_array.nil? || (id_or_array.is_a?(Array) && id_or_array.empty?)

delete_by(primary_key => id_or_array)
end

Expand Down
11 changes: 11 additions & 0 deletions activerecord/test/cases/persistence_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,17 @@ def test_class_level_delete
assert_nothing_raised { Reply.find(should_not_be_destroyed_reply.id) }
end

def test_class_level_delete_with_invalid_ids
assert_no_queries do
assert_equal 0, Topic.delete(nil)
assert_equal 0, Topic.delete([])
end

assert_difference -> { Topic.count }, -1 do
assert_equal 1, Topic.delete(topics(:first).id)
end
end

def test_class_level_delete_is_affected_by_scoping
should_not_be_destroyed_reply = Reply.create("title" => "hello", "content" => "world")
Topic.find(1).replies << should_not_be_destroyed_reply
Expand Down

0 comments on commit 55bf2df

Please sign in to comment.