New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
merge parent joins for through associations #24682
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -134,13 +134,13 @@ def add_constraints(scope, owner, association_klass, refl, chain_head, chain_tai | |
scope = next_chain_scope(scope, table, reflection, association_klass, foreign_table, next_reflection) | ||
end | ||
|
||
# Exclude the scope of the association itself, because that | ||
# was already merged in the #scope method. | ||
reflection.constraints.each do |scope_chain_item| | ||
item = eval_scope(reflection.klass, scope_chain_item, owner) | ||
item = eval_scope(reflection.klass, scope_chain_item, owner) | ||
|
||
if scope_chain_item == refl.scope | ||
scope.merge! item.except(:where, :includes) | ||
elsif item.joins_values.any? | ||
merge_joins(scope, item) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this different than the merge that will occur in the call to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That doesn't appear to be the case. The code is almost exactly identical to what you have here. https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation/merger.rb#L107-L129 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it looks pretty much the same, but there are differences at the end. Instead of passing |
||
end | ||
|
||
reflection.all_includes do | ||
|
@@ -158,6 +158,21 @@ def add_constraints(scope, owner, association_klass, refl, chain_head, chain_tai | |
scope | ||
end | ||
|
||
# Merges inner joins from `other` relation to `scope` relation. | ||
def merge_joins(scope, other) | ||
association_joins, rest_joins = other.joins_values.partition do |join| | ||
case join | ||
when Hash, Symbol, Array | ||
true | ||
else | ||
false | ||
end | ||
end | ||
|
||
join_dependency = ActiveRecord::Associations::JoinDependency.new(other.klass, association_joins, rest_joins) | ||
scope.joins! join_dependency.join_constraints([], Arel::Nodes::InnerJoin).map(&:joins) | ||
end | ||
|
||
def eval_scope(klass, scope, owner) | ||
klass.unscoped.instance_exec(owner, &scope) | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outdated comment