do not load all child records for inverse case

currently `post.comments.find(Comment.first.id)` would load all
comments for the given post to set the inverse association.

This has a huge performance penalty. Because if post has 100k
records and all these 100k records would be loaded in memory
even though the comment id was supplied.

Fix is to use in-memory records only if loaded? is true. Otherwise
load the records using full sql.

Fixes #10509
This commit is contained in:
Neeraj Singh 2013-05-11 00:34:25 -04:00
parent 353a398bee
commit 2b73f780ff
3 changed files with 25 additions and 1 deletions

@ -1,3 +1,19 @@
* Do not load all child records for inverse case.
currently `post.comments.find(Comment.first.id)` would load all
comments for the given post to set the inverse association.
This has a huge performance penalty. Because if post has 100k
records and all these 100k records would be loaded in memory
even though the comment id was supplied.
Fix is to use in-memory records only if loaded? is true. Otherwise
load the records using full sql.
Fixes #10509.
*Neeraj Singh*
* Fixture setup does no longer depend on `ActiveRecord::Base.configurations`.
This is relevant when `ENV["DATABASE_URL"]` is used in place of a `database.yml`.

@ -81,7 +81,7 @@ def find(*args)
else
if options[:finder_sql]
find_by_scan(*args)
elsif options[:inverse_of]
elsif options[:inverse_of] && loaded?
args = args.flatten
raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args.blank?

@ -401,6 +401,14 @@ def test_parent_instance_should_find_child_instance_using_child_instance_id_when
assert_equal man.name, man.interests.find(interest.id).man.name, "The name of the man should match after the child name is changed"
end
def test_find_on_child_instance_with_id_should_not_load_all_child_records
man = Man.create!
interest = Interest.create!(man: man)
man.interests.find(interest.id)
refute man.interests.loaded?
end
def test_raise_record_not_found_error_when_invalid_ids_are_passed
man = Man.create!