mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
While merging relations preserve context for joins
Fixes #3002. Also see #5494. ``` class Comment < ActiveRecord::Base belongs_to :post end class Author < ActiveRecord::Base has_many :posts end class Post < ActiveRecord::Base belongs_to :author has_many :comments end ``` `Comment.joins(:post).merge(Post.joins(:author).merge(Author.where(:name => "Joe Blogs"))).all` would fail with `ActiveRecord::ConfigurationError: Association named 'author' was not found on Comment`. It is failing because `all` is being called on relation which looks like this after all the merging: `{:joins=>[:post, :author], :where=>[#<Arel::Nodes::Equality: ....}`. In this relation all the context that `Post` was joined with `Author` is lost and hence the error that `author` was not found on `Comment`. Ths solution is to build JoinAssociation when two relations with join information are being merged. And later while building the arel use the previously built `JoinAssociation` record in `JoinDependency#graft` to build the right from clause. Thanks to Jared Armstrong (https://github.com/armstrjare) for most of the work. I ported it to make it compatible with new code base.
This commit is contained in:
parent
bb61d2defa
commit
dc764fcc34
4 changed files with 78 additions and 4 deletions
|
@ -1,5 +1,37 @@
|
|||
## Rails 4.0.0 (unreleased) ##
|
||||
|
||||
* Preserve context while merging relations with join information.
|
||||
|
||||
class Comment < ActiveRecord::Base
|
||||
belongs_to :post
|
||||
end
|
||||
|
||||
class Author < ActiveRecord::Base
|
||||
has_many :posts
|
||||
end
|
||||
|
||||
class Post < ActiveRecord::Base
|
||||
belongs_to :author
|
||||
has_many :comments
|
||||
end
|
||||
|
||||
`Comment.joins(:post).merge(Post.joins(:author).merge(Author.where(:name => "Joe Blogs"))).all`
|
||||
would fail with
|
||||
`ActiveRecord::ConfigurationError: Association named 'author' was not found on Comment`.
|
||||
|
||||
It is failing because `all` is being called on relation which looks like this after all
|
||||
the merging: `{:joins=>[:post, :author], :where=>[#<Arel::Nodes::Equality: ....}`. In this
|
||||
relation all the context that `Post` was joined with `Author` is lost and hence the error
|
||||
that `author` was not found on `Comment`.
|
||||
|
||||
Ths solution is to build JoinAssociation when two relations with join information are being
|
||||
merged. And later while building the arel use the previously built `JoinAssociation` record
|
||||
in `JoinDependency#graft` to build the right from clause.
|
||||
|
||||
Fixes #3002.
|
||||
|
||||
*Jared Armstrong and Neeraj Singh*
|
||||
|
||||
* `default_scopes?` is deprecated. Check for `default_scopes.empty?` instead.
|
||||
|
||||
*Agis Anastasopoulos*
|
||||
|
|
|
@ -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.base_klass == join_part.base_klass
|
||||
else
|
||||
parent == join_part
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -39,7 +39,7 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
class Merger # :nodoc:
|
||||
attr_reader :relation, :values
|
||||
attr_reader :relation, :values, :other
|
||||
|
||||
def initialize(relation, other)
|
||||
if other.default_scoped? && other.klass != relation.klass
|
||||
|
@ -48,11 +48,12 @@ module ActiveRecord
|
|||
|
||||
@relation = relation
|
||||
@values = other.values
|
||||
@other = other
|
||||
end
|
||||
|
||||
NORMAL_VALUES = Relation::SINGLE_VALUE_METHODS +
|
||||
Relation::MULTI_VALUE_METHODS -
|
||||
[:where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc:
|
||||
[:joins, :where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc:
|
||||
|
||||
def normal_values
|
||||
NORMAL_VALUES
|
||||
|
@ -66,12 +67,39 @@ module ActiveRecord
|
|||
|
||||
merge_multi_values
|
||||
merge_single_values
|
||||
merge_joins
|
||||
|
||||
relation
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def merge_joins
|
||||
return if values[:joins].blank?
|
||||
|
||||
if other.klass == relation.klass
|
||||
relation.joins! *values[:joins]
|
||||
else
|
||||
joins_dependency, rest = values[:joins].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! rest
|
||||
|
||||
join_dependency.join_associations.each do |association|
|
||||
@relation = association.join_relation(relation)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def merge_multi_values
|
||||
relation.where_values = merged_wheres
|
||||
relation.bind_values = merged_binds
|
||||
|
|
|
@ -1,10 +1,12 @@
|
|||
require "cases/helper"
|
||||
require 'models/post'
|
||||
require 'models/comment'
|
||||
require 'models/author'
|
||||
require 'models/rating'
|
||||
|
||||
module ActiveRecord
|
||||
class RelationTest < ActiveRecord::TestCase
|
||||
fixtures :posts, :comments
|
||||
fixtures :posts, :comments, :authors
|
||||
|
||||
class FakeKlass < Struct.new(:table_name, :name)
|
||||
end
|
||||
|
@ -176,6 +178,13 @@ module ActiveRecord
|
|||
relation.merge!(where: ['foo = ?', 'bar'])
|
||||
assert_equal ['foo = bar'], relation.where_values
|
||||
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 3, authors(:david).posts.merge(posts_with_special_comments_with_ratings).to_a.length
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
class RelationMutationTest < ActiveSupport::TestCase
|
||||
|
|
Loading…
Reference in a new issue