Skip to content

Commit

Permalink
Ignore empty condition on #construct_relation_for_exists
Browse files Browse the repository at this point in the history
At fc0e335,

```rb
relation = relation.where(conditions)
```

was rewritten to:

```rb
relation.where!(condition)
```

This change accidentally changed the result of `Topic.exists?({})` from true to false.

To fix this regression, first I moved the blank check logic (`opts.blank?`) from `#where` to `#where!`,
because I thought `#where!` should be identical to `#where`, except that instead of returning a new relation,
it adds the condition to the existing relation.

But on second thought after some discussion on #34329,
I started to think that just fixing `#construct_relation_for_exists` is more preferable
than changing `#where` and `#where!`.
  • Loading branch information
r7kamura committed Oct 27, 2018
1 parent 5431e17 commit 4694fcf
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation/finder_methods.rb
Expand Up @@ -363,7 +363,7 @@ def construct_relation_for_exists(conditions)

case conditions
when Array, Hash
relation.where!(conditions)
relation.where!(conditions) unless conditions.empty?
else
relation.where!(primary_key => conditions) unless conditions == :none
end
Expand Down
4 changes: 4 additions & 0 deletions activerecord/test/cases/finder_test.rb
Expand Up @@ -246,6 +246,10 @@ def test_exists_with_nil_arg
assert_equal true, Topic.first.replies.exists?
end

def test_exists_with_empty_hash_arg
assert_equal true, Topic.exists?({})
end

# Ensure +exists?+ runs without an error by excluding distinct value.
# See https://github.com/rails/rails/pull/26981.
def test_exists_with_order_and_distinct
Expand Down

0 comments on commit 4694fcf

Please sign in to comment.