Skip to content

Commit

Permalink
Should work inverse association when eager loading
Browse files Browse the repository at this point in the history
This regression was caused by caa178c. The block for
`set_inverse_instance` should also be passed to join dependency.

Fixes #30402.
  • Loading branch information
kamipo committed Aug 29, 2017
1 parent 595a231 commit 237f00c
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,15 @@ def join_constraints(outer_joins, join_type)
end

def aliases
Aliases.new join_root.each_with_index.map { |join_part, i|
@aliases ||= Aliases.new join_root.each_with_index.map { |join_part, i|
columns = join_part.column_names.each_with_index.map { |column_name, j|
Aliases::Column.new column_name, "t#{i}_r#{j}"
}
Aliases::Table.new(join_part, columns)
}
end

def instantiate(result_set, aliases)
def instantiate(result_set, &block)
primary_key = aliases.column_alias(join_root, join_root.primary_key)

seen = Hash.new { |i, object_id|
Expand All @@ -157,7 +157,7 @@ def instantiate(result_set, aliases)
message_bus.instrument("instantiation.active_record", payload) do
result_set.each { |row_hash|
parent_key = primary_key ? row_hash[primary_key] : row_hash
parent = parents[parent_key] ||= join_root.instantiate(row_hash, column_aliases)
parent = parents[parent_key] ||= join_root.instantiate(row_hash, column_aliases, &block)
construct(parent, join_root, row_hash, result_set, seen, model_cache, aliases)
}
end
Expand Down
16 changes: 14 additions & 2 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ def to_sql
relation = self

if eager_loading?
find_with_associations { |rel| relation = rel }
find_with_associations { |rel, _| relation = rel }
end

conn = klass.connection
Expand Down Expand Up @@ -664,7 +664,19 @@ def has_join_values?
end

def exec_queries(&block)
@records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes, &block).freeze
@records =
if eager_loading?
find_with_associations do |relation, join_dependency|
if ActiveRecord::NullRelation === relation
[]
else
rows = connection.select_all(relation.arel, "SQL", relation.bound_attributes)
join_dependency.instantiate(rows, &block)
end.freeze
end
else
klass.find_by_sql(arel, bound_attributes, &block).freeze
end

preload = preload_values
preload += includes_values unless eager_loading?
Expand Down
12 changes: 1 addition & 11 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -371,17 +371,7 @@ def find_with_associations
relation = select aliases.columns
relation = apply_join_dependency(relation, join_dependency)

if block_given?
yield relation
else
if ActiveRecord::NullRelation === relation
[]
else
arel = relation.arel
rows = connection.select_all(arel, "SQL", relation.bound_attributes)
join_dependency.instantiate(rows, aliases)
end
end
yield relation, join_dependency
end

def construct_relation_for_exists(relation, conditions)
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/cases/associations/eager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ def test_eager_with_has_one_through_join_model_with_conditions_on_the_through
assert_nil member.favourite_club
end

def test_should_work_inverse_of_with_eager_load
author = authors(:david)
assert_same author, author.posts.first.author
assert_same author, author.posts.eager_load(:comments).first.author
end

def test_loading_with_one_association
posts = Post.all.merge!(includes: :comments).to_a
post = posts.find { |p| p.id == 1 }
Expand Down

0 comments on commit 237f00c

Please sign in to comment.