Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extract construct_relation_for_exists in FinderMethods #28705

Merged
merged 2 commits into from
Apr 8, 2017
Merged

Extract construct_relation_for_exists in FinderMethods #28705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a76f518
More exercise `exists?` tests
kamipo Apr 8, 2017
0da696a
Extract `construct_relation_for_exists` in `FinderMethods`
kamipo Apr 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 15 additions & 12 deletions activerecord/lib/active_record/relation/finder_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,16 +312,7 @@ def exists?(conditions = :none)
relation = apply_join_dependency(self, construct_join_dependency(eager_loading: false))
return false if ActiveRecord::NullRelation === relation

relation = relation.except(:select, :distinct).select(ONE_AS_ONE).limit(1)

case conditions
when Array, Hash
relation = relation.where(conditions)
else
unless conditions == :none
relation = relation.where(primary_key => conditions)
end
end
relation = construct_relation_for_exists(relation, conditions)

connection.select_value(relation, "#{name} Exists", relation.bound_attributes) ? true : false
rescue ::RangeError
Expand Down Expand Up @@ -391,6 +382,19 @@ def find_with_associations
end
end

def construct_relation_for_exists(relation, conditions)
relation = relation.except(:select, :distinct)._select!(ONE_AS_ONE).limit!(1)

case conditions
when Array, Hash
relation.where!(conditions)
else
relation.where!(primary_key => conditions) unless conditions == :none
end

relation
end

def construct_join_dependency(joins = [], eager_loading: true)
including = eager_load_values + includes_values
ActiveRecord::Associations::JoinDependency.new(@klass, including, joins, eager_loading: eager_loading)
Expand All @@ -401,8 +405,7 @@ def construct_relation_for_association_calculations
end

def apply_join_dependency(relation, join_dependency)
relation = relation.except(:includes, :eager_load, :preload)
relation = relation.joins join_dependency
relation = relation.except(:includes, :eager_load, :preload).joins!(join_dependency)

if using_limitable_reflections?(join_dependency.reflections)
relation
Expand Down
23 changes: 18 additions & 5 deletions activerecord/test/cases/finder_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,24 @@ def test_exists_with_nil_arg
assert_equal true, Topic.first.replies.exists?
end

# ensures +exists?+ runs valid SQL by excluding order value
def test_exists_with_order
# 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
assert_equal true, Topic.order(:id).distinct.exists?
end

def test_exists_with_joins
assert_equal true, Topic.joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists?
end

def test_exists_with_left_joins
assert_equal true, Topic.left_joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists?
end

def test_exists_with_eager_load
assert_equal true, Topic.eager_load(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists?
end

def test_exists_with_includes_limit_and_empty_result
assert_equal false, Topic.includes(:replies).limit(0).exists?
assert_equal false, Topic.includes(:replies).limit(1).where("0 = 1").exists?
Expand Down Expand Up @@ -236,9 +249,9 @@ def test_exists_with_aggregate_having_three_mappings

def test_exists_with_aggregate_having_three_mappings_with_one_difference
existing_address = customers(:david).address
assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city, existing_address.country + "1"))
assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city + "1", existing_address.country))
assert_equal false, Customer.exists?(address: Address.new(existing_address.street + "1", existing_address.city, existing_address.country))
assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city, existing_address.country + "1"))
assert_equal false, Customer.exists?(address: Address.new(existing_address.street, existing_address.city + "1", existing_address.country))
assert_equal false, Customer.exists?(address: Address.new(existing_address.street + "1", existing_address.city, existing_address.country))
end

def test_exists_does_not_instantiate_records
Expand Down