Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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.
  • Loading branch information...
commit c09829e03db611b46bb52e2054991222cf57bfbe 1 parent 5c6cf4e
@ahorner ahorner authored
View
5 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*
View
2  activerecord/lib/active_record/associations/join_dependency.rb
@@ -109,7 +109,7 @@ def build(associations, parent = nil, join_type = Arel::InnerJoin)
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)
View
7 activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -55,7 +55,12 @@ def ==(other)
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
View
28 activerecord/lib/active_record/relation/spawn_methods.rb
@@ -27,7 +27,7 @@ def merge(r)
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 @@ def apply_finder_options(options)
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
View
9 activerecord/test/cases/relations_test.rb
@@ -4,6 +4,7 @@
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 @@
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 @@ def test_relation_merging_with_joins
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
Please sign in to comment.
Something went wrong with that request. Please try again.