Preload Associations - bug

Hi guys,

I came across this obscure bug in the new preload associations:

NoMethodError (undefined method `preload_associations' for NilClass:Class):     /vendor/rails/activerecord/lib/active_record/ association_preload.rb:25:in `preload_associations'     /vendor/rails/activerecord/lib/active_record/ association_preload.rb:19:in `each'     /vendor/rails/activerecord/lib/active_record/ association_preload.rb:19:in `preload_associations'     /vendor/rails/activerecord/lib/active_record/ association_preload.rb:16:in `preload_associations'     /vendor/rails/activerecord/lib/active_record/ association_preload.rb:16:in `each'     /vendor/rails/activerecord/lib/active_record/ association_preload.rb:16:in `preload_associations'     /vendor/rails/activerecord/lib/active_record/base.rb:1247:in `find_every'     /vendor/rails/activerecord/lib/active_record/base.rb:502:in `find' ....

This occurs when I pass a two parameters in an :order statement, at least this is the only way I've come across the issue and been able to replicate it.

HumanResource::Person.find(:all, :include => [:person_position => :position], :order => 'last_name') -- works fine

HumanResource::Person.find(:all, :include => [:person_position => :position], :order => 'last_name,first_name') -- causes error above

However, there is more to the problem, I don't believe it revolves around the :order parameter after looking at the association_preload.rb code. When I step through the code on line 23: parents = records.map {|record| record.send(reflection.name)}.flatten

parents will return [nil, #<HumanResource::Person::Position id: 2, person_id: 10001, position_id: 4, void_cheque: 0, hired_on: nil, terminated_on: nil>]

The issue is when the preload of the association takes place, one of the models returns nothing, the first element, and this creates an error on line 25. To fix this, I simply added ".compact" to the end of line 23. Unfortunately I don't have experience messing around with the Rails core or writing unit tests for it so I wasn't able to formalize this issue.

Regards,

Peter

That sounds about right (blame me, I wrote all that stuff :-). There's actually already a patch at http://dev.rubyonrails.org/attachment/ticket/9640 that addresses this and some other issues.

Fred

Hi Fred,

I looked over your patch, and its been sometime now that its been sitting in the pending patches, what is taking so long for it to be merged with the trunk? It's certainly an issue for anyone using edge and eager loading.

Regards,

Peter

I'm a little confused what the status is of the patches attached to 9640, perhaps consolidating them into another patch ticket for review would save everyone some time and hassle?

Hi Fred,

I looked over your patch, and its been sometime now that its been sitting in the pending patches, what is taking so long for it to be merged with the trunk? It's certainly an issue for anyone using edge and eager loading.

I'm a little confused what the status is of the patches attached to 9640, perhaps consolidating them into another patch ticket for review would save everyone some time and hassle?

That would be a plan. None of those patches are mine (and i've been on
holiday so I'm completely out of it) It looks like blweiner's http://dev.rubyonrails.org/attachment/ticket/9640/fix_nested_includes2.patch   and http://dev.rubyonrails.org/attachment/ticket/9640/fix_nested_includes.patch   should be just one patch.

blweiner, if you're out there could you chuck those in their own ticket?

Fred

Who is in charge here? Presumably, many people; after all that is the open source way. However, like any engineering team with a collective goal there must be order and structure. But it seems we have left that up to a bug tracking system. We build new features faster then we fix old bugs. Bugs begin to build upon other bugs. DHH a few months back told me that the 37signals apps run from an almost bleeding edge version of rails. That was before 2.0 was released. I do the same with some of my apps today, and its been stable.

Naysayers who don't see the light, constantly pass Rails off as technology lacking robustness. Its just a tool to quickly mockup a website -- make a blog in 5 minutes. Ouch, those are the kinds of developers that learn Java in college or PHP from a web tutorial and proclaim mastery. Rails developers are programmers that are passionate about their craft. They understand the art form and continually push technology.

But now .. the bug in this posting, which will affect anyone including two associations, has been neglected for almost a month. What irritates me is that a patch has been ready hours after the bug was discovered, yet the five letters ".chop" at the end of line 23 still have not been amended. Obviously it was forgotten in the shuffle of more exciting work such as building new features. But features are thrown out the window if you cant maintain robustness in the process.

Regards,

Peter

But now .. the bug in this posting, which will affect anyone including two associations, has been neglected for almost a month. What irritates me is that a patch has been ready hours after the bug was discovered, yet the five letters ".chop" at the end of line 23 still have not been amended. Obviously it was forgotten in the shuffle of more exciting work such as building new features. But features are thrown out the window if you cant maintain robustness in the process.

Peter,

As I mentioned earlier in this thread this ticket now has more than 20 patches, and more than 50 comments, it's impossible for someone new to this issue to figure out what is going on, let alone review the patches and apply them.

What's needed are *new* tickets for each of the bugs found, so that the review and commit process is simpler for everyone concerned. As someone who clearly knows what's going on, perhaps you could create a new ticket, and upload the relevant patches to it? Two weeks ago I mentioned the best way forward to getting these patches applied, frederick followed up with some more information. Let's do that rather than getting indignant about new features and robustness.