From 55f7be7ef91e82571d74c0fcf59fabb87d7ba513 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Fri, 11 Oct 2019 14:06:13 +0900 Subject: [PATCH] Fix eager load for no :has_many with limit and joins for :has_many The results is deduplicated when eager loading. However, eager loading for no :has_many with limit and joins for :has_many do not deal with this while eager loading for :has_many deals with this. Fixes #37356 --- activerecord/lib/active_record/relation/finder_methods.rb | 3 ++- activerecord/test/cases/finder_test.rb | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index a08c46d58e..29efc44b9c 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -384,7 +384,8 @@ def apply_join_dependency(eager_loading: group_values.empty?) ) relation = except(:includes, :eager_load, :preload).joins!(join_dependency) - if eager_loading && !using_limitable_reflections?(join_dependency.reflections) + reflections = join_dependency.reflections + joins_values.map { |joins_value| reflect_on_association(joins_value) }.reject(&:blank?) + if eager_loading && !using_limitable_reflections?(reflections) if has_limit_or_offset? limited_ids = limited_ids_for(relation) limited_ids.empty? ? relation.none! : relation.where!(primary_key => limited_ids) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 1e482481a7..d732f740da 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1336,6 +1336,12 @@ def test_with_limiting_with_custom_select assert_equal [0, 1, 1], posts.map(&:author_id).sort end + def test_eager_load_for_no_has_many_with_limit_and_joins_for_has_many + relation = Post.eager_load(:author).joins(:comments) + assert_equal 5, relation.to_a.size + assert_equal relation.limit(5).to_a.size, relation.to_a.size + end + def test_find_one_message_on_primary_key e = assert_raises(ActiveRecord::RecordNotFound) do Car.find(0)