has_many :through => belongs_to_association bug

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

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.

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.

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 

+1 Tested on mysql. Approach looks sound.

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

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,

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,

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.

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

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?

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.

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

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

Hey Zach,

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

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

Pratik,

Any chance you can a look at this?

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.

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?

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

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.

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

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