Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Mark Van Holstyn June 3rd, 2008 @ 09:16 PM

  +1 tests all passing and looks like a valid use of the has_many :through

• You flagged this item as spam.
  

  Zach Dennis June 3rd, 2008 @ 09:16 PM

  The above output is:

  SQLite3::SQLException: no such column: trips.trip_id: SELECT "expenses".* FROM "expenses" INNER JOIN trips ON expenses.trip_id = trips.id WHERE (("trips".trip_id = 1)
  

  The additional conditions was from testing different variations until I found the source of the problem.

• You flagged this item as spam.
  

  Zach Dennis June 3rd, 2008 @ 10:21 PM

  Attached is an updated patch. Rather than relying on fixtures this patch creates what it needs inside the test. This forces the implementation to be be updated as well to be correct.

  Apply the attached patch "has_many_through_belongs_to_association.2.diff" by itself, not on top of the original patch. If you have applied the original patch then reset that and apply the latest patch.

  • has_many_through_belongs_to_association.2.diff 3.7 KB

• You flagged this item as spam.
  

  Zach Dennis June 4th, 2008 @ 12:05 AM

  Attached is another update to the patch to cover the below scenario:

  class Day
    belongs_to :trip
    has_many :expenses, :through => :trip
  end
  
  # - this would blow up since it has no trip, but it should return an empty array
  # - this patch has this return an empty array if the day doesn't have a trip_id
  Day.new.expenses 
  

  • has_many_through_belongs_to_association.3.diff 4.7 KB

• You flagged this item as spam.
  

  Gabe da Silveira June 4th, 2008 @ 12:58 AM

  +1 Tested on mysql. Approach looks sound.

• 

  Frederick Cheung June 9th, 2008 @ 09:06 AM

  I believe you will need to make some changes to that you can eager load such an association.

• 

  Pratik June 9th, 2008 @ 11:23 AM

  • State changed from “new” to “incomplete”
  • Assigned user set to “Pratik”

  What fred said. Could you add tests for preloading as well ?

  Thanks.

• You flagged this item as spam.
  

  Zach Dennis June 13th, 2008 @ 03:18 AM

  I'm working on updating the patch to incorporate a test for eager loading. I think some refactoring will be underway since the way has many through associations work assumes the structure of a has_one or has_many structure, and using a belongs_to changes things around, at least in regard to what table has the primary key to use, what the primary key field is, etc.

  Diligently working on it though,

• You flagged this item as spam.
  

  Zach Dennis June 13th, 2008 @ 04:27 AM

  The eager loading test is working, but I need to go back and refactor now that everything is working and I understand the code a little better. Here's the current WIP state of things:

  http://github.com/zdennis/rails/...

  I'll post an updated patch this weekend. Thanks,

• You flagged this item as spam.
  

  Zach Dennis June 20th, 2008 @ 03:52 AM

  Attached is an updated patch (which replaces the previous ones, don't use them anymore).

  This adds test for eager loading and introduces a simple strategy object for has many associations so the conditional logic for choosing a primary key name or primary key are limited to a single spot.

  The association preloading could be refactored into using different preload objects which handle preloading records based on the type of association. This would clean up and simplify how preloading is done IMO. I almost went that route until Craig Demyanovich suggested going the strategy route first since it was much simpler and had less impact on the overall preloading code.

  • has_many_through_belongs_to_association.4.diff 9.4 KB

• You flagged this item as spam.
  

  Zach Dennis July 10th, 2008 @ 08:22 PM

  • Tag set to “activerecord, bug, patch”

  Ping, any love on this pratik?

• 

  Pratik July 10th, 2008 @ 08:40 PM

  • State changed from “incomplete” to “open”

• You flagged this item as spam.
  

  Glenn Powell August 7th, 2008 @ 04:46 PM

  I believe the 4th patch is out of date. Is anyone working on integrating a fix into the core?

• You flagged this item as spam.
  

  Glenn Powell August 11th, 2008 @ 08:15 AM

  This does seem like a fairly important bug to be resolved ASAP. I personally continuously run into a brick wall regarding this, since I opted to bypass the outdated patch and await a core fix (but to no avail). What is the status of this now?

• You flagged this item as spam.
  

  Zach Dennis August 11th, 2008 @ 01:30 PM

  I know core is a busy group of folks and when they're going to give this patch a look I'll update it again. All they need to do is ping the ticket.

• 

  Frederick Cheung August 11th, 2008 @ 01:57 PM

  Can't hurt to ping the rails-core list.

• 

  Zach Dennis August 18th, 2008 @ 09:06 PM

  • no changes were found...

  • has_many_through_belongs_to_association.5.diff 129 KB

• You flagged this item as spam.
  

  Zach Dennis August 18th, 2008 @ 09:06 PM

  Here's an updated patch to work against Rails today. Please include.

• 

  Pratik August 19th, 2008 @ 05:51 PM

  Hey Zach,

  I see a lot of tests are commented in your latest patch. Is that intentional ?

• You flagged this item as spam.
  

  Zach Dennis August 19th, 2008 @ 06:29 PM

  Sorry about that. I squashed my several commits into one clean commit. See the latest diff. Thanks,

  • has_many_through_belongs_to_association.6.diff 9.4 KB

• You flagged this item as spam.
  

  Zach Dennis August 23rd, 2008 @ 03:19 PM

  Pratik,

  Any chance you can a look at this?

• 

  Pratik August 29th, 2008 @ 12:18 PM

  Hey,

  Sorry for the delay. I don't like this patch a lot. So I've been trying to put this logic inside reflection. Turns out, that'll need me a little more time.

  Thanks.

• 

  Pratik September 15th, 2008 @ 10:13 AM

  • Milestone cleared.
• 

  Repository October 4th, 2008 @ 05:51 PM

  • State changed from “open” to “resolved”

  (from [95e1cf4812d4b964d7ab0fdf4bfa31177d27909c]) Fix has_many :through when the source is a belongs_to association. [#323 state:resolved]

  Signed-off-by: Pratik Naik pratiknaik@gmail.com http://github.com/rails/rails/co...

• You flagged this item as spam.
  

  Glenn Powell February 2nd, 2009 @ 04:06 PM

  I'm actually still seeing something which I believe is a result of this problem. (I am on 2.2, perhaps it's fixed in 2.3)

  I have this relationship:

  
  class Foo < ActiveRecord::Base
  end
  
  class Bar < ActiveRecord::Base
    belongs_to :foo
  end
  
  class Bat < ActiveRecord::Base
    belongs_to :bar
    has_one :foo, :through => :bar
  end
  

  If I call the through attribute directly, then it works:

  
  >> Bat.first.foo
  => #<Foo id:1>
  

  But if I try to include the :foo association in any find conditions, then I get a similar error to the one above.

  
  >> Bat.all(:include => :foo, :conditions => { 'foos.id' => 1 })
  ActiveRecord::StatementInvalid: Mysql::Error: Unknown column 'bars.bar_id' in 'on clause': SELECT `bats`.`id` AS t0_r0, `bat`.`bar_id` AS t0_r1, `foos`.`id` AS t1_r0 FROM `bats`  LEFT OUTER JOIN `bars` ON (`bats`.`id` = `bars`.`bar_id`)  LEFT OUTER JOIN `foos` ON (`foos`.`id` = `bats`.`foo_id`) WHERE (`foos`.`id` = 1) 
  

  It seems as though the join on condition (bats.id = bars.bar_id) has simply reversed the foreign/primary keys here. So it should be: bats.bar_id = bars.id Correct?

• 

  Frederick Cheung February 2nd, 2009 @ 04:13 PM

  I definitely remember a patch not too long ago to do with :include in a similar case - certainly worth checking out 2.3

• You flagged this item as spam.
  

  Glenn Powell February 2nd, 2009 @ 06:08 PM

  Yes, now I've gone ahead and tried it out in Rails Edge 2.3, but it still fails with the same error.

  Frederick, do you still have a link to that patch? I'd be interested to check it out.

• 

  Frederick Cheung February 2nd, 2009 @ 06:11 PM

  Not off the top of my head. Now that I think about it, was has_one :though that was looking at.

• You flagged this item as spam.
  

  Corey February 10th, 2009 @ 09:03 PM

  Any update on the eager loading in a single query case of 2.2?