Address shortcomings of changeset [8054]

rubyonrails-core
1

Hey folks,

for those of you interested in the new :joins => :associations features, can you please take a look at this patch:

http://dev.rubyonrails.org/ticket/10061

Rather than futzing around with ":joins => :associations gets loaded like :include does only we discard the included records" and having to pepper that logic throughout AciveRecord, this patch does something simpler and more consistent:

It changes add_joins!() to turn all non-string :joins arguments into a list of inner joins based on the [:associations, :you, :supply].

No other hidden magic, normal :joins semantics apply:

Swimmers.find(:all, :select => 'distinct swimmers.*', :joins => :goggles)

is the same as:

Swimmers.find(:all, :select => 'distinct swimmers.*', :joins => 'inner join goggles on goggles.swimmer_id = swimmers.id')

And the following behaves as one would expect:

Swimmers.find(:all, :include => :goggles, :joins => :noseplugs, :conditions => ['noseplugs.color = ?', 'red']) # load all swimmers and their goggles, as long as they have a red noseplug.

What I'm looking for is comments that "yeah, that's how it should be working" before I work through the tests that originally came with changeset 8054 (they assume :join => :stuff should behave like :include rather than like :join).

I have no idea what I'll do if it turns into an angry-dance-off between the "Includys" and the "Joinys".

Thanks for taking the time to look at this. Trevor

2

Awesome! I had been meaning to do that.

Actually, I think this patch should be done in two parts.

1. Undo everything done by 8054, except docs. because..

- It fires same big fat query as eager loading ( select t1.c1...t2.c2... instead of select t.* ) - I also don't really like the way it uses same code as eager loading. - Extremely high memory usage because of returned result is m*n order, just like eager loading - Check comments on my blog post at http://m.onkey.org/2007/11/1/find-users-with-at-least-n-items

2. Implement same functionality in add_joins!

Further, I also think it should, by default, fire "SELECT DISTINCT table.* ..." query and may be have an extra option to say :distinct => false to override that.

Probably, join query building code should be moved to reflection class from JoinAssociation.

So yeah, +1 for the idea.

3

Hey,

There's definitely a discussion with the whole 'distinct' thing but right now I'm mostly concerned with addressing the way :include and :joins is confounded at present.

If everyone wanted it as such, I could submit as 2 patches, but as it stands my single patch undoes the confounding while still preserving the feature of :joins => {:some => :associations} becoming an implicit list of inner joins.

I've commented on the ticket - http://dev.rubyonrails.org/ticket/10061 - with more information in response to Rick's issues.

I'd really love it if folks could +/- 1 on the *strategy* that :include means 'include', :joins means 'joins'.

If I get enough concensus on the strategy (and buy-in from a core member), I'll prepare a patch (or 2) according to whatever guidelines are laid out by the core member.

It would be cool if this could be dusted off before too many new changes are applied to the files in question.

Thanks for taking the time to look at this.

Trev