diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index f187835bb0a..2682917bece 100644 --- a/activerecord/CHANGELOG.md +++ b/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, diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 920bf2da871..975f2035afa 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -190,12 +190,12 @@ module ActiveRecord 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 @@ module ActiveRecord 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 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 bd87870a3eb..de4d5f643af 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -25,8 +25,9 @@ module ActiveRecord 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 diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index 7bab2c2afed..39dba1eed7e 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -12,6 +12,10 @@ require "models/tagging" 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 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase 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 diff --git a/activerecord/test/cases/associations/left_outer_join_association_test.rb b/activerecord/test/cases/associations/left_outer_join_association_test.rb index 02ccdeb1a15..b6a340e5282 100644 --- a/activerecord/test/cases/associations/left_outer_join_association_test.rb +++ b/activerecord/test/cases/associations/left_outer_join_association_test.rb @@ -9,6 +9,10 @@ require "models/essay" 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 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase 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 diff --git a/activerecord/test/models/friendship.rb b/activerecord/test/models/friendship.rb index 9f1712a8ec5..d1e49d544bf 100644 --- a/activerecord/test/models/friendship.rb +++ b/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 diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index 581157ac56c..f8b59bbc6ba 100644 --- a/activerecord/test/models/person.rb +++ b/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