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.