named_scope :order on association with :order is unintuitive
Reported by Henrik Nyh | May 19th, 2008 @ 10:55 AM
If I have
class Foo < ActiveRecord::Base
has_many :bars, :order => 'one'
end
class Bar < ActiveRecord::Base
named_scope :by_two, :order => 'two'
named_scope :by_three, :order => 'three'
end
then
>>> a_foo.bars.by_two
will generate SQL containing "ORDER BY one, two".
This is unintuitive to me: I would either expect to override the order entirely (ORDER BY two) or for the last scope to have precedence (ORDER BY two, one).
>>> a_foo.bars.by_two.by_three
will generate the same SQL: "ORDER BY one, two".
This is unintuitive as well; I would expect it to either use the last given order (ORDER by three) or all orders, with right-to-left precedence (ORDER BY three, two, one).
Comments and changes to this ticket
-
Joshua Peek July 16th, 2008 @ 07:50 PM
- → Assigned user changed from “” to “Pratik”
- → Tag changed from “” to “activerecord enhancement named_scope”
-
Henrik Nyh July 17th, 2008 @ 05:00 PM
Related issue:
If you have a named scope with a :select, doing
Foo.that_named_scope.find(:all, :select => 'something')
will use the select from the named scope, not the one in the find.
-
David Turnbull August 22nd, 2008 @ 03:58 AM
- no changes were found...
-
David Turnbull August 22nd, 2008 @ 03:58 AM
I've come across similar.. in my case I didn't get a comma separated order statement, just the first order param passed in the scope chain.
The attached patch seems to address this issue. It specifically deals with :order alongside :includes, :conditions, and merges them with the last ordering being most important.
-
Pratik August 22nd, 2008 @ 07:10 AM
- → State changed from “new” to “incomplete”
Patch is missing tests.
Thanks.
-
-
Henrik Nyh August 25th, 2008 @ 03:23 AM
David, thanks for doing this.
Confirmed that the tests pass.
It does not, however, seem to handle the case where the association is defined with an order, like "has_many :bars, :order => 'one'" above.
Attaching a failing test to this effect that goes on top of David's latest patch.
Not saying the patch should not be accepted without this -- better to fix a little than nothing at all -- but if it's easy to get this, too, that would rock.
-
David Turnbull August 25th, 2008 @ 10:07 AM
Henrik,
You can get the test working by ordering by 'comments.body', and changing the add_order! function in active_record/base.rb, as attached. It assumes you'll want to apply a named scope on an association..
This breaks using a finder on a named scope, though.
-
Henrik Nyh August 25th, 2008 @ 10:23 AM
David: Thanks! "breaks using a finder on a named scope" only in the sense that the :order from named scopes will always beat the order from associations/finds, right?
So basically one has to choose whether to let associations/finds or named scopes get priority?
Seems like one could modify the calls to add_order! (or check the caller, but that's nasty), so we can tell whether the non-scope order is from an association or a find.
Basically, the add_order! calls from associations.rb could be passed a forth parameter like "association = true", and then association orders could be lower prio than scope orders, but find orders would be higher prio.
Does this make sense? Is it too complicated? I personally would much prefer moderate complication if it means ActiveRecord behaves more intuitively.
I'd be happy to try this out and submit a patch, just wanted to check if it makes sense to someone more familiar with the codebase.
-
David Turnbull August 25th, 2008 @ 04:30 PM
Yes, the named scope ordering beats associations/finds with my changes.
One problem being that in our tests, it's only base.rb#construct_finder_sql that calls add_order! and you don't know much context from there.
I think you're probably right about the priority being dynamic finder < named scope < association ordering.
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Source available from github
The Git repository resides at http://github.com/rails
Check out the current development trunk (Edge Rails) with:
git clone git://github.com/rails/rails.git
The latest development for the 1.2.x and 2.0.x releases are on the 1-2-stable and 2-0-stable branches.
Creating a bug report
When creating a bug report, be sure to include as much relevant information as possible. Post the code sample that causes the problem. Preferably, alter the unit tests and show through either changed or added tests how the expected behavior is not occuring.
Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use trac for reporting security vulnerabilities. All content in trac is publicly available as soon as it is posted.
Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kinda bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to sign on to with a "I'm having this problem too".
