Skip to content
This repository
Browse code

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
  • Loading branch information...
commit 9f1f89b0f372cb36a1d989cc1dd7bb2257578ec4 1 parent 010bdec
thedarkone authored
16 activerecord/CHANGELOG.md
Source Rendered
... ... @@ -1,5 +1,21 @@
1 1 ## unreleased ##
2 2
  3 +* Do not load all child records for inverse case.
  4 +
  5 + currently `post.comments.find(Comment.first.id)` would load all
  6 + comments for the given post to set the inverse association.
  7 +
  8 + This has a huge performance penalty. Because if post has 100k
  9 + records and all these 100k records would be loaded in memory
  10 + even though the comment id was supplied.
  11 +
  12 + Fix is to use in-memory records only if loaded? is true. Otherwise
  13 + load the records using full sql.
  14 +
  15 + Fixes #10509.
  16 +
  17 + *Neeraj Singh*
  18 +
3 19 * `ActiveRecord::FinderMethods#exists?` returns `true`/`false` in all cases.
4 20
5 21 *Xavier Noria*
2  activerecord/lib/active_record/associations/collection_association.rb
@@ -81,7 +81,7 @@ def find(*args)
81 81 else
82 82 if options[:finder_sql]
83 83 find_by_scan(*args)
84   - elsif options[:inverse_of]
  84 + elsif options[:inverse_of] && loaded?
85 85 args = args.flatten
86 86 raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args.blank?
87 87
8 activerecord/test/cases/associations/inverse_associations_test.rb
@@ -319,6 +319,14 @@ def test_parent_instance_should_find_child_instance_using_child_instance_id_when
319 319 assert_equal man.name, man.interests.find(interest.id).man.name, "The name of the man should match after the child name is changed"
320 320 end
321 321
  322 + def test_find_on_child_instance_with_id_should_not_load_all_child_records
  323 + man = Man.create!
  324 + interest = Interest.create!(man: man)
  325 +
  326 + man.interests.find(interest.id)
  327 + refute man.interests.loaded?
  328 + end
  329 +
322 330 def test_raise_record_not_found_error_when_invalid_ids_are_passed
323 331 # delete all interest records to ensure that hard coded invalid_id(s)
324 332 # are indeed invalid.

0 comments on commit 9f1f89b

Please sign in to comment.
Something went wrong with that request. Please try again.