Fix .left_outer_joins when multiple associations have the same child.

Signed-off-by: Garrett Blehm <[email protected]>

Author: garrettblehm

activerecord/CHANGELOG.md

@@ -1,3 +1,14 @@
+*   Fix an issue where `.left_outer_joins` used with multiple associations that have
+    the same child association but different parents does not join all parents.
+
+    Previously, using `.left_outer_joins` with the same child association would only join one of the parents.
+
+    Now it will correctly join both parents.
+
+    Fixes #41498.
+
+    *Garrett Blehm*
+
 *   Deprecate `unsigned_float` and `unsigned_decimal` short-hand column methods.
 
     As of MySQL 8.0.17, the UNSIGNED attribute is deprecated for columns of type FLOAT, DOUBLE,

activerecord/lib/active_record/associations/join_dependency.rb

@@ -190,12 +190,12 @@ def make_join_constraints(join_root, join_type)
         def make_constraints(parent, child, join_type)
           foreign_table = parent.table
           foreign_klass = parent.base_klass
-          child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) do |reflection|
-            table, terminated = @joined_tables[reflection]
+          child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) do |reflection, remaining_reflection_chain|
+            table, terminated = @joined_tables[remaining_reflection_chain]
             root = reflection == child.reflection
 
             if table && (!root || !terminated)
-              @joined_tables[reflection] = [table, root] if root
+              @joined_tables[remaining_reflection_chain] = [table, root] if root
               next table, true
             end
 
@@ -206,7 +206,7 @@ def make_constraints(parent, child, join_type)
               root ? name : "#{name}_join"
             end
 
-            @joined_tables[reflection] ||= [table, root] if join_type == Arel::Nodes::OuterJoin
+            @joined_tables[remaining_reflection_chain] ||= [table, root] if join_type == Arel::Nodes::OuterJoin
             table
           end.concat child.children.flat_map { |c| make_constraints(child, c, join_type) }
         end

activerecord/lib/active_record/associations/join_dependency/join_association.rb

@@ -25,8 +25,9 @@ def join_constraints(foreign_table, foreign_klass, join_type, alias_tracker)
           joins = []
           chain = []
 
-          reflection.chain.each do |reflection|
-            table, terminated = yield reflection
+          reflection_chain = reflection.chain
+          reflection_chain.each_with_index do |reflection, index|
+            table, terminated = yield reflection, reflection_chain[index..]
             @table ||= table
 
             if terminated

activerecord/test/cases/associations/inner_join_association_test.rb

@@ -12,6 +12,10 @@
 require "models/tag"
 require "models/sharded/blog_post"
 require "models/sharded/comment"
+require "models/friendship"
+require "models/reader"
+require "models/reference"
+require "models/job"
 
 class InnerJoinAssociationTest < ActiveRecord::TestCase
   fixtures :authors, :author_addresses, :essays, :posts, :comments, :categories, :categories_posts, :categorizations,
@@ -231,4 +235,12 @@ def test_find_with_conditions_on_through_reflection
     assert_not_empty blog_posts
     assert_equal(expected_comment.blog_post, blog_posts.first)
   end
+
+  def test_inner_joins_includes_all_nested_associations
+    queries = capture_sql { Friendship.joins(:friend_favorite_reference_job, :follower_favorite_reference_job).to_a }
+    # Match mysql and postgresql/sqlite quoting
+    quote = Regexp.union(%w[" `])
+    assert queries.any? { |sql| /#{quote}friendships#{quote}.#{quote}friend_id#{quote}/i.match?(sql) }
+    assert queries.any? { |sql| /#{quote}friendships#{quote}.#{quote}follower_id#{quote}/i.match?(sql) }
+  end
 end

activerecord/test/cases/associations/left_outer_join_association_test.rb

@@ -9,6 +9,10 @@
 require "models/category"
 require "models/categorization"
 require "models/person"
+require "models/friendship"
+require "models/reader"
+require "models/reference"
+require "models/job"
 
 class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
   fixtures :authors, :author_addresses, :essays, :posts, :comments, :ratings, :categorizations, :people
@@ -120,4 +124,12 @@ def test_does_not_override_select
 
     assert_equal [author], Author.where(id: author).left_outer_joins(:special_categorizations)
   end
+
+  def test_left_outer_joins_includes_all_nested_associations
+    queries = capture_sql { Friendship.left_outer_joins(:friend_favorite_reference_job, :follower_favorite_reference_job).to_a }
+    # Match mysql and postgresql/sqlite quoting
+    quote = Regexp.union(%w[" `])
+    assert queries.any? { |sql| /#{quote}friendships#{quote}.#{quote}friend_id#{quote}/i.match?(sql) }
+    assert queries.any? { |sql| /#{quote}friendships#{quote}.#{quote}follower_id#{quote}/i.match?(sql) }
+  end
 end

activerecord/test/models/friendship.rb

@@ -5,4 +5,7 @@ class Friendship < ActiveRecord::Base
   # friend_too exists to test a bug, and probably shouldn't be used elsewhere
   belongs_to :friend_too, foreign_key: "friend_id", class_name: "Person", counter_cache: :friends_too_count
   belongs_to :follower, class_name: "Person"
+
+  has_one :friend_favorite_reference_job, through: :friend, source: :favorite_reference_job
+  has_one :follower_favorite_reference_job, through: :follower, source: :favorite_reference_job
 end

activerecord/test/models/person.rb

@@ -18,7 +18,8 @@ class Person < ActiveRecord::Base
   has_many :references
   has_many :bad_references
   has_many :fixed_bad_references, -> { where favorite: true }, class_name: "BadReference"
-  has_one  :favorite_reference, -> { where "favorite=?", true }, class_name: "Reference"
+  has_one  :favorite_reference, -> { where favorite: true }, class_name: "Reference"
+  has_one :favorite_reference_job, through: :favorite_reference, source: :job
   has_many :posts_with_comments_sorted_by_comment_id, -> { includes(:comments).order("comments.id") }, through: :readers, source: :post
   has_many :first_posts, -> { where(id: [1, 2]) }, through: :readers