aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorNeeraj Singh <neerajdotname@gmail.com>2013-05-11 00:34:25 -0400
committerNeeraj Singh <neerajdotname@gmail.com>2013-06-19 04:09:43 +0530
commit2b73f780ffa52baba09511b2db753f0fde574c14 (patch)
tree8948c673b922e14008623dfb7d27692d5fa18d5b /activerecord
parent353a398bee68c5ea99d76ac7601de0a5fef6f4a5 (diff)
downloadrails-2b73f780ffa52baba09511b2db753f0fde574c14.tar.gz
rails-2b73f780ffa52baba09511b2db753f0fde574c14.tar.bz2
rails-2b73f780ffa52baba09511b2db753f0fde574c14.zip
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
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md16
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb2
-rw-r--r--activerecord/test/cases/associations/inverse_associations_test.rb8
3 files changed, 25 insertions, 1 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 963cdadba5..2f85a2ddae 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -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`.
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index efd7ecb97c..9833822f8f 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -81,7 +81,7 @@ module ActiveRecord
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?
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index b1f0be3204..993e7294cf 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -401,6 +401,14 @@ class InverseHasManyTests < ActiveRecord::TestCase
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!