Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

[patch] has many through eager loading problem

#2362

A has_many through relationship with conditions will load as if the conditions don't exist when eager loading.

To replicate:

Apply the following patch to rails source. It adds a has_many through (HMT) relationship with conditions to the post model and a test that shows the wrong thing is loaded when eager loading is used

diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 4072381..e902637 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -352,6 +352,12 @@ class EagerAssociationTest < ActiveRecord::TestCase
     end
   end

+  def test_eager_with_has_many_through_with_conditions_join_model_with_include
+    post_tags = Post.find(posts(:welcome).id).misc_tags
+    eager_post_tags = Post.find(posts(:welcome).id, :include => :misc_tags).misc_tags
+    assert_equal post_tags, eager_post_tags
+  end
+
   def test_eager_with_has_many_and_limit
     posts = Post.find(:all, :order => 'posts.id asc', :include => [ :author, :comments ], :limit => 2)
     assert_equal 2, posts.size
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 374e536..1691666 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -52,6 +52,7 @@ class Post < ActiveRecord::Base
     end
   end

+  has_many :misc_tags, :through => :taggings, :source => :tag, :conditions => "tags.name = 'Misc'"
   has_many :funky_tags, :through => :taggings, :source => :tag
   has_many :super_tags, :through => :taggings
   has_one :tagging, :as => :taggable

Results of test

C:\local\rails\activerecord
14:27:03.48>rake test_sqlite3 TEST=test\cases\associations\eager_test.rb
(in C:/local/rails/activerecord)
c:/local/ruby/bin/ruby -I"C:/local/rails/activerecord/lib" -I"C:/local/rails/activerecord/test" -I"C:/local/rails/activerecord/test/
connections/native_sqlite3" "c:/local/ruby/lib/ruby/gems/1.8/gems/rake-0.8.4/lib/rake/rake_test_loader.rb" "test\cases\associations\
eager_test.rb"
Using native SQLite3
Loaded suite c:/local/ruby/lib/ruby/gems/1.8/gems/rake-0.8.4/lib/rake/rake_test_loader
Started
.........................................................F..........................................
Finished in 2.297 seconds.

  1) Failure:
test_eager_with_has_many_through_with_conditions_join_model_with_include(EagerAssociationTest)
    [./test\cases\associations\eager_test.rb:358:in `test_eager_with_has_many_through_with_conditions_join_model_with_include'
     C:/local/rails/activerecord/test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in `__send__'

     C:/local/rails/activerecord/test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in `run']:
<[]> expected but was
<[#<Tag id: 1, name: "General", taggings_count: 0>]>.

100 tests, 322 assertions, 1 failures, 0 errors
rake aborted!
Command failed with status (1): [c:/local/ruby/bin/ruby -I"C:/local/rails/a...]

Reported by Roy W. Black · March 27th, 2009 @ 06:34 PM

State: committed
Milestone: 3.0.2
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. Andrew Roth
  2. Daniel X Moore
    Daniel X Moore

    +1

    I'm having the same problem.

    ←[4;35;1mProduct Load (126.0ms)←[0m ←[0mSELECT * FROM products WHERE (products.id IN (468,193,457,369,567,469,458,546,216,568,194,249,470,459,250, 195,471,218,460,515,251,196,461,516,197,561,462,550,495,198,617,584,496,463,573,452,199,519,453,365,497,200,520,366,454,521,543,367,477,466,279,510,499,455, 191,522,478,467,357,280,192,5,511,456) AND (products.status_id = 2 AND products.inventory_type_id = 1)) ←[0m ←[4;36;1mProduct Load (5.0ms)←[0m ←[0;1mSELECT * FROM products WHERE (products.id = 495) ←[0m ←[4;35;1mProduct Load (4.0ms)←[0m ←[0mSELECT * FROM products WHERE (products.id = 499) ←[0m ←[4;36;1mProduct Load (4.0ms)←[0m ←[0;1mSELECT * FROM products WHERE (products.id = 515) ←[0m

    Those extra three that load individually don't match the conditions.

    April 17th, 2009 @ 09:12 PM

  3. Daniel X Moore
    Daniel X Moore

    I have a checked out copy of rails source, the applied patch, and the failing test. I would like to contribute a fix for this, but I need a little help getting pointed in the right direction.

    I'm looking at active_record/association_preload.rb around preload_has_many_association, but I'm having trouble really getting to the bottom of things. If anyone can link me to a guide about ActiveRecord internals then I would greatly appreciate it.

    Thanks for any help.

    April 21st, 2009 @ 12:55 AM

  4. Cristi Prodan
  5. Rizwan Reza
    Rizwan Reza
    • Milestone changed from 2.x to 2.3.6

    March 27th, 2010 @ 09:25 AM

  6. Rizwan Reza
    Rizwan Reza
    • Tag changed from 2.3.2, associations to 2.3.2, associations, bugmash

    May 16th, 2010 @ 02:41 AM

  7. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.6 to 2.3.7

    May 23rd, 2010 @ 05:54 PM

  8. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.7 to 2.3.8

    May 24th, 2010 @ 09:40 AM

  9. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.8 to 2.3.9

    May 25th, 2010 @ 11:45 PM

  10. Grant Ammons
    Grant Ammons
    • Importance changed from to Low

    The issue is that loading the association is not taking into account the :conditions when loading the :through association.

    Created a patch that will join the associated model and apply the conditions to the :through query. This is the only real way I can see fixing this, because as it stands right now, AR will grab the associated :through model(s) first, and then grab the association, but by then it's too late!

    June 28th, 2010 @ 09:59 PM

  11. Grant Ammons
  12. Neeraj Singh
    Neeraj Singh
    • State changed from new to open

    @Grant I could not apply this patch against 2-3-stable branch. Did you create the patch against 2-3-x or rails master?

    June 28th, 2010 @ 10:52 PM

  13. Grant Ammons
    Grant Ammons

    Oh, that patch is against master. Attached is a patch for 2-3-stable.

    June 29th, 2010 @ 12:01 AM

  14. Neeraj Singh
    Neeraj Singh
    • Assigned user set to Pratik

    +1 I applied the fix for rails master and it works fine. Did not try 2-3-stable fix but that looks good too.

    June 29th, 2010 @ 02:58 AM

  15. Grant Ammons
    Grant Ammons
    • Title changed from has many through eager loading problem to [patch] has many through eager loading problem

    July 4th, 2010 @ 03:39 PM

  16. José Valim
    José Valim
    • Assigned user changed from Pratik to José Valim
    • Milestone cleared.

    July 7th, 2010 @ 06:23 PM

  17. Repository
    Repository
    • State changed from open to resolved

    (from [17650e394fae26984d506fd0f705bc32e5a5de27]) Eager loading :through associations will join the :source model if there are :conditions. [#2362 [patch] has many through eager loading problem state:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/17650e394fae26984d506fd0f705bc...

    July 8th, 2010 @ 10:03 PM

  18. Repository
    Repository
    • State changed from resolved to committed

    (from [0963774c0a6a58ba13ac0ff4763527ea7d994c0a]) fixes #2362 [patch] has many through eager loading problem, eager loading :through associations will join the :source model if there are :conditions

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/0963774c0a6a58ba13ac0ff4763527...

    July 8th, 2010 @ 10:11 PM

  19. Jeremy Kemper