Fix for obscure bug with eager-loading and nested includes

rubyonrails-core
1

Hi all,

Here’s a patch for fixing a bug with nested includes and missing data when you use the old-style single query eager loading.

http://rails.lighthouseapp.com/projects/8994/tickets/1696

It’s a fairly obscure bug; we only saw it in a live 1.2.6 app after it’d been out for about a year and with 2.x’s multi-query eager loading for most cases it’s even less likely to occur, but it’s a bug none-the-less. What it boils down to is: doing a find with 3 nested includes where the first include has no data will cause a NoMethodError. The problem is that JoinDependency uses 2 arrays (associaitons and joins) to interrogate rows of data and assumes they never get out of sync, which in this case, they do. The fix is to find the correct join in the second array for the association we are constructing, rather than just using the first join in the array.

I’d appreciate some more eyes on it as I’m not hugely convinced that the test case is complete / expressive enough.

Cheers,

Muz

2

Hi all,

Here's a patch for fixing a bug with nested includes and missing
data when you use the old-style single query eager loading.

#1696 Problem with nested include missing data and old-style single query eager loading - Ruby on Rails - rails

It's a fairly obscure bug; we only saw it in a live 1.2.6 app after
it'd been out for about a year and with 2.x's multi-query eager
loading for most cases it's even less likely to occur, but it's a
bug none-the-less. What it boils down to is: doing a find with 3
nested includes where the first include has no data will cause a
NoMethodError. The problem is that JoinDependency uses 2 arrays
(associaitons and joins) to interrogate rows of data and assumes
they never get out of sync, which in this case, they do. The fix is
to find the correct join in the second array for the association we
are constructing, rather than just using the first join in the array.

I'd appreciate some more eyes on it as I'm not hugely convinced that
the test case is complete / expressive enough.

Ah JoinDependency, my least favourite bit of rails :slight_smile: I'll have a
look at this one.

Fred