Skip to content

Commit

Permalink
Make relation.exists? more performant when using eager loading
Browse files Browse the repository at this point in the history
`relation.exists?` just wants to know if there is a result or not, does
not need the exact records matched. Therefore, an intermediate SELECT
query for eager loading is not necessary.
  • Loading branch information
kamipo committed Jan 10, 2018
1 parent 96f5930 commit b4f64cb
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 13 deletions.
7 changes: 4 additions & 3 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def exists?(conditions = :none)
return false if !conditions || limit_value == 0

if eager_loading?
relation = apply_join_dependency(construct_join_dependency(eager_loading: false))
relation = apply_join_dependency(eager_loading: false)
return relation.exists?(conditions)
end

Expand Down Expand Up @@ -396,10 +396,11 @@ def construct_join_dependency(eager_loading: true)
)
end

def apply_join_dependency(join_dependency = construct_join_dependency)
def apply_join_dependency(join_dependency = nil, eager_loading: true)
join_dependency ||= construct_join_dependency(eager_loading: eager_loading)
relation = except(:includes, :eager_load, :preload).joins!(join_dependency)

if using_limitable_reflections?(join_dependency.reflections)
if !eager_loading || using_limitable_reflections?(join_dependency.reflections)
relation
else
if has_limit_or_offset?
Expand Down
22 changes: 12 additions & 10 deletions activerecord/test/cases/finder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,25 +270,27 @@ def test_exists_with_eager_load
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?
assert_no_queries { assert_equal false, Topic.includes(:replies).limit(0).exists? }
assert_queries(1) { assert_equal false, Topic.includes(:replies).limit(1).where("0 = 1").exists? }
end

def test_exists_with_distinct_association_includes_and_limit
author = Author.first
assert_equal false, author.unique_categorized_posts.includes(:special_comments).limit(0).exists?
assert_equal true, author.unique_categorized_posts.includes(:special_comments).limit(1).exists?
unique_categorized_posts = author.unique_categorized_posts.includes(:special_comments)
assert_no_queries { assert_equal false, unique_categorized_posts.limit(0).exists? }
assert_queries(1) { assert_equal true, unique_categorized_posts.limit(1).exists? }
end

def test_exists_with_distinct_association_includes_limit_and_order
author = Author.first
assert_equal false, author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC").limit(0).exists?
assert_equal true, author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC").limit(1).exists?
unique_categorized_posts = author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC")
assert_no_queries { assert_equal false, unique_categorized_posts.limit(0).exists? }
assert_queries(1) { assert_equal true, unique_categorized_posts.limit(1).exists? }
end

def test_exists_should_reference_correct_aliases_while_joining_tables_of_has_many_through_association
developer = developers(:david)
assert_not_predicate developer.ratings.includes(comment: :post).where(posts: { id: 1 }), :exists?
ratings = developers(:david).ratings.includes(comment: :post).where(posts: { id: 1 })
assert_queries(1) { assert_not_predicate ratings.limit(1), :exists? }
end

def test_exists_with_empty_table_and_no_args_given
Expand Down Expand Up @@ -1315,12 +1317,12 @@ def test_finder_with_offset_string

test "#skip_query_cache! for #exists? with a limited eager load" do
Topic.cache do
assert_queries(2) do
assert_queries(1) do
Topic.eager_load(:replies).limit(1).exists?
Topic.eager_load(:replies).limit(1).exists?
end

assert_queries(4) do
assert_queries(2) do
Topic.eager_load(:replies).limit(1).skip_query_cache!.exists?
Topic.eager_load(:replies).limit(1).skip_query_cache!.exists?
end
Expand Down

0 comments on commit b4f64cb

Please sign in to comment.