Merge pull request #11948 from thedarkone/backport-11049-11051-4-0-st…

…able Backport #11049 and #11051 into 4-0-stable
rails · Aug 20, 2013 · a3bf24a · a3bf24a
2 parents ec1227a + 9f1f89b
commit a3bf24a
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 2 deletions.
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
@@ -1,5 +1,21 @@
 ## unreleased ##
 
+*   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*
+
 *   `ActiveRecord::FinderMethods#exists?` returns `true`/`false` in all cases.
 
     *Xavier Noria*

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
@@ -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?
 

diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -319,10 +319,22 @@ 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
+    # delete all interest records to ensure that hard coded invalid_id(s)
+    # are indeed invalid.
+    Interest.delete_all
+
     man = Man.create!
 
-    invalid_id = 2394823094892348920348523452345
+    invalid_id = 245324523
     assert_raise(ActiveRecord::RecordNotFound) { man.interests.find(invalid_id) }
 
     invalid_ids = [8432342, 2390102913, 2453245234523452]