Why not merge :joins in with_scope?

rubyonrails-talk
1

Hi,

We're chaining several named_scope calls like this:

Post.something_with_a_join.another_with_a_join

And we found out the :joins in the scope aren't merged.

I'm wondering if there's any reason why :joins aren't merged but :conditions and :include are, see: http://github.com/rails/rails/tree/master/activerecord/lib/active_record/base.rb#L1823

I know some join could be contradictory but they can also be required by the conditions.

I have a patch ready for this but I'm waiting for some feedback before writing some tests and submitting it, maybe I'm missing something.

Thanks, Marc

2

Hi,

We're chaining several named_scope calls like this:

Post.something_with_a_join.another_with_a_join

And we found out the :joins in the scope aren't merged.

I'm wondering if there's any reason why :joins aren't merged but :conditions and :include are, see: http://github.com/rails/rails/tree/master/activerecord/lib/active_record/base.rb#L1823

:joins comes in 2 forms. When :joins is like :include (ie an array of
(possibly nested) association names, i don't see why it couldn't
behave like :include. That usage is newish (last autumn?) so it could
jus be an omission. In the other usage where :joins is an sql fragment
then it's rather more problematic (for doing things like removing
duplicates etc...)

Fred

3

Shouldn't the DB server intelligent enough to filter out duplicates in that case ?

Thanks! François

4

You're right, but by not merging :joins it fails in both cases: when it's like include and as an sql fragment

If we merge the :joins like described here: http://blog.teksol.info/2008/5/26/why-are-activerecord-scopes-not-merged , it will only fail in the later case, when it's used a an sql fragment and both conflict.

5

Doesn't sound like the sort of thing you could rely on across the board.

6

You're right, but by not merging :joins it fails in both cases: when it's like include and as an sql fragment

If we merge the :joins like described here: http://blog.teksol.info/2008/5/26/why-are-activerecord-scopes-not-merged , it will only fail in the later case, when it's used a an sql fragment and both conflict.

I think you'd probably want to base yourself on what :include does (in
fact you could probably reuse the merge_includes code), as you need to
handle the fact that you can say :joins => :foo or :joins => [:foo]
and things like that

Fred

7

Hey,

I'm probably the last person to touch the :joins => :association style syntax so I'd like to chime in with a warning here. The change proposed in:

http://blog.teksol.info/2008/5/26/why-are-activerecord-scopes-not-merged

sets off alarm bells for me. See the following line:

http://github.com/rails/rails/tree/master/activerecord/lib/active_record/base.rb#L1552

What that line does is ensure that your joins {:like => :this}, :or, [:like, :this] will be treated as an *inner* join. If you just concatenate the :joins together into a string you'll break those semantics.

I know Fred has already brought it to your attention that you should base your code off the way that :includes are handled (merge_includes) but I just wanted to add some weight to that.

And as for "why doesn't it already do what it ought to?"... I'm pretty sure that when I reworked that stuff I didn't actually *remove* anything so I kind of hope it never merged joins in scopes in the first place... if I did remove something critical then I shall quietly flog myself :stuck_out_tongue:

Regards, Trevor