From c09829e03db611b46bb52e2054991222cf57bfbe Mon Sep 17 00:00:00 2001 From: Andrew Horner Date: Thu, 18 Apr 2013 23:41:58 -0500 Subject: Preserve context for joins while merging relations This is a backport of #10164, already merged into master. The issue is described in lengthy detail in issues #3002 and #5494. --- activerecord/CHANGELOG.md | 5 ++++ .../active_record/associations/join_dependency.rb | 2 +- .../join_dependency/join_association.rb | 7 +++++- .../lib/active_record/relation/spawn_methods.rb | 28 +++++++++++++++++++++- activerecord/test/cases/relations_test.rb | 9 ++++++- 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d27b11bddd..aa561bb8d3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,10 @@ ## unreleased ## +* Maintain context for joins within ActiveRecord::Relation merges. + Backport #10164. + + *Neeraj Singh + Andrew Horner* + * Make sure the `EXPLAIN` command is never triggered by a `select_db` call. *Daniel Schierbeck* diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index cd366ac8b7..e3d8356f49 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -109,7 +109,7 @@ module ActiveRecord case associations when Symbol, String reflection = parent.reflections[associations.to_s.intern] or - raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?" + raise ConfigurationError, "Association named '#{ associations }' was not found on #{parent.active_record.name}; perhaps you misspelled it?" unless join_association = find_join_association(reflection, parent) @reflections << reflection join_association = build_join_association(reflection, parent) 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 03963ab060..becf1a3f62 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -55,7 +55,12 @@ module ActiveRecord def find_parent_in(other_join_dependency) other_join_dependency.join_parts.detect do |join_part| - parent == join_part + case parent + when JoinBase + parent.active_record == join_part.active_record + else + parent == join_part + end end end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index b8e384c2f3..90f2ac3cde 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -27,7 +27,7 @@ module ActiveRecord merge_relation_method(merged_relation, method, value) if value.present? end - merged_relation.joins_values += r.joins_values + merge_joins(merged_relation, r) merged_wheres = @where_values + r.where_values @@ -146,6 +146,32 @@ module ActiveRecord private + def merge_joins(relation, other) + values = other.joins_values + return if values.blank? + + if other.klass == relation.klass + relation.joins_values += values + else + joins_dependency, rest = values.partition do |join| + case join + when Hash, Symbol, Array + true + else + false + end + end + + join_dependency = ActiveRecord::Associations::JoinDependency.new( + other.klass, + joins_dependency, + [] + ) + + relation.joins_values += join_dependency.join_associations + rest + end + end + def merge_relation_method(relation, method, value) relation.send(:"#{method}_values=", relation.send(:"#{method}_values") + value) end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 98fe6b7611..f14eee2eb8 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -4,6 +4,7 @@ require 'models/tagging' require 'models/post' require 'models/topic' require 'models/comment' +require 'models/rating' require 'models/reply' require 'models/author' require 'models/comment' @@ -19,7 +20,7 @@ require 'models/minivan' class RelationTest < ActiveRecord::TestCase fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments, - :tags, :taggings, :cars, :minivans + :ratings, :tags, :taggings, :cars, :minivans def test_do_not_double_quote_string_id van = Minivan.last @@ -696,6 +697,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal 1, comments.count end + def test_relation_merging_with_merged_joins + special_comments_with_ratings = SpecialComment.joins(:ratings) + posts_with_special_comments_with_ratings = Post.group('posts.id').joins(:special_comments).merge(special_comments_with_ratings) + assert_equal 1, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count.length + end + def test_count posts = Post.scoped -- cgit v1.2.3