From 6a617cc61ee8bd83159a61c83dbb2f71a384ddae Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 21 May 2020 23:58:19 +0900 Subject: [PATCH] Fix through association with source/through scope which has joins If source/through scope references other tables in where/order, we should explicitly maintain joins in the scope, otherwise association queries will fail with referenced unknown column. Fixes #33525. --- .../lib/active_record/associations/association_scope.rb | 6 ++++++ .../associations/join_dependency/join_association.rb | 7 +++++++ .../lib/active_record/associations/through_association.rb | 2 +- activerecord/lib/active_record/relation/finder_methods.rb | 2 +- .../associations/has_many_through_associations_test.rb | 8 ++++++++ activerecord/test/fixtures/readers.yml | 6 ++++++ activerecord/test/models/author.rb | 1 + 7 files changed, 30 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 23ac979567..608603f298 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -134,6 +134,12 @@ module ActiveRecord if scope_chain_item == chain_head.scope scope.merge! item.except(:where, :includes, :unscope, :order) + elsif !item.references_values.empty? + join_dependency = item.construct_join_dependency( + item.eager_load_values | item.includes_values, Arel::Nodes::OuterJoin + ) + scope.joins!(*item.joins_values, join_dependency) + scope.left_outer_joins!(*item.left_outer_joins_values) end reflection.all_includes do diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 3c66ce1866..31d12fa2fa 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -33,6 +33,13 @@ module ActiveRecord join_scope = reflection.join_scope(table, foreign_table, foreign_klass) + unless join_scope.references_values.empty? + join_dependency = join_scope.construct_join_dependency( + join_scope.eager_load_values | join_scope.includes_values, Arel::Nodes::OuterJoin + ) + join_scope.joins!(join_dependency) + end + arel = join_scope.arel(alias_tracker.aliases) nodes = arel.constraints.first diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 15e6565e69..3f5e9066eb 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -32,7 +32,7 @@ module ActiveRecord reflection.chain.drop(1).each do |reflection| relation = reflection.klass.scope_for_association scope.merge!( - relation.except(:select, :create_with, :includes, :preload, :joins, :eager_load) + relation.except(:select, :create_with, :includes, :preload, :eager_load, :joins, :left_outer_joins) ) end scope diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 839a68b299..c1062a390c 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -380,7 +380,7 @@ module ActiveRecord def apply_join_dependency(eager_loading: group_values.empty?) join_dependency = construct_join_dependency( - eager_load_values + includes_values, Arel::Nodes::OuterJoin + eager_load_values | includes_values, Arel::Nodes::OuterJoin ) relation = except(:includes, :eager_load, :preload).joins!(join_dependency) diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index ed9ae1df5f..284075fb5c 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1057,10 +1057,18 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_has_many_through_with_source_scope expected = [readers(:michael_welcome).becomes(LazyReader)] + assert_equal expected, Author.first.lazy_readers_skimmers_or_not assert_equal expected, Author.preload(:lazy_readers_skimmers_or_not).first.lazy_readers_skimmers_or_not assert_equal expected, Author.eager_load(:lazy_readers_skimmers_or_not).first.lazy_readers_skimmers_or_not end + def test_has_many_through_with_join_scope + expected = [readers(:bob_welcome).becomes(LazyReader)] + assert_equal expected, Author.last.lazy_readers_skimmers_or_not_2 + assert_equal expected, Author.preload(:lazy_readers_skimmers_or_not_2).last.lazy_readers_skimmers_or_not_2 + assert_equal expected, Author.eager_load(:lazy_readers_skimmers_or_not_2).last.lazy_readers_skimmers_or_not_2 + end + def test_has_many_through_polymorphic_with_rewhere post = TaggedPost.create!(title: "Tagged", body: "Post") tag = post.tags.create!(name: "Tag") diff --git a/activerecord/test/fixtures/readers.yml b/activerecord/test/fixtures/readers.yml index 14b883f041..246d94ffaa 100644 --- a/activerecord/test/fixtures/readers.yml +++ b/activerecord/test/fixtures/readers.yml @@ -9,3 +9,9 @@ michael_authorless: post_id: 3 person_id: 1 first_post_id: 3 + +bob_welcome: + id: 3 + post_id: 8 + person_id: 4 + first_post_id: 10 diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index a44479e858..1fba8ae493 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -177,6 +177,7 @@ class Author < ActiveRecord::Base has_many :topics, primary_key: "name", foreign_key: "author_name" has_many :lazy_readers_skimmers_or_not, through: :posts + has_many :lazy_readers_skimmers_or_not_2, through: :posts_with_no_comments, source: :lazy_readers_skimmers_or_not attr_accessor :post_log after_initialize :set_post_log