Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Inconsistent behavior using "order" in scopes

#5479

Using Rails 3.0RC2 (it happened in RC1 as well) with a SQLite3 database I've noticed the following behavior under both Ruby 1.8.7 and Ruby 1.9.2.

I've got a post class with the following scopes:

    scope :by_id, order("id")
    scope :by_title, order("title")

I see the following queries are generated by combining those scopes and the "order" method:

    Post.by_title.by_id
    SELECT "posts".* FROM "posts" ORDER BY id

    Post.order("title").order("id")
    SELECT "posts".* FROM "posts" ORDER BY title, id

    Post.order("title").by_id
    SELECT "posts".* FROM "posts" ORDER BY id

    Post.by_title.order("id")
    SELECT "posts".* FROM "posts" ORDER BY title, id

I don't think this is what most people would expect. I'd expect that using "by_id" and "order('id')" would be equivalent, but is not. Looks like the declared scope overrides the order but the "order" method chains the columns.

I'll try to investigate a bit further and provide a test case, but don't expect anything soon.

P.S: sorry if it's a duplicated bug. I've seen similar bugs about default scope and order, but thought were not the same.

Reported by javierm · August 27th, 2010 @ 07:44 PM

State: committed
Milestone: 3.0.2
Assigned to: Santiago Pastorino Santiago Pastorino
Importance: Low

Activity

  1. javierm
    javierm

    Sooner than I expected, here's the failing test.

    August 27th, 2010 @ 11:05 PM

  2. Neeraj Singh
    Neeraj Singh
    • Milestone set to 3.x
    • State changed from new to open
    • Assigned user set to Neeraj Singh
    • Tag set to rails 3
    • Importance changed from to Low

    August 27th, 2010 @ 11:48 PM

  3. Neeraj Singh
    Neeraj Singh

    I have a fix for this one. However my fix is breaking a number of tests. Here I am discussing one of the tests that is failing because of my fix.

    Let's start from the beginning. We have a model which looks like this.

    class DeveloperOrderedBySalary < ActiveRecord::Base
      self.table_name = 'developers'
      default_scope :order => 'salary DESC'
      scope :by_name, :order => 'name DESC'
      scope :reordered_by_name, reorder('name DESC')
    
      def self.all_ordered_by_name
        with_scope(:find => { :order => 'name DESC' }) do
          find(:all)
        end
      end
    end
    

    Now the question is what kind of sql should be generated for the following case

    DeveloperOrderedBySalary.all_ordered_by_name
    

    The existing rails edge code would produce following sql

    SELECT "developers".* FROM "developers" ORDER BY name DESC
    

    I also happen to think that above sql is wrong. It is wrong because it does not take into account the default_scope. default_scope is always applied as the very first thing. And if the default_scope dictates order then that order should be followed. Notice that with_scope obeys default_scope in general, but in this case with_cope is NOT obeying default_scope.

    After my fix the sql generated is

    SELECT "developers".* FROM "developers" ORDER BY salary DESC, name DESC
    

    As I said earlier my fix for this issue is breaking a lot of tests similar to the case illustrated. I would like to have an opinion from a core team member on this topic.

    August 28th, 2010 @ 04:49 AM

  4. Neeraj Singh
    Neeraj Singh

    Current behavior in rails 2-3-stable is

    class User < ActiveRecord::Base
      default_scope  :order => 'created_at desc'
    
      named_scope :custom, :order => 'name desc'
    end
    
    > User.all
    SELECT * FROM "users" ORDER BY created_at desc
    
    > User.custom
    SELECT * FROM "users" ORDER BY name desc
    

    August 28th, 2010 @ 10:12 PM

  5. Santiago Pastorino
    Santiago Pastorino
    • Milestone cleared.
    • Assigned user changed from Neeraj Singh to Santiago Pastorino

    Neeraj only to write a bit of the progress we did on this. We have a patch and Neeraj is writing some tests.

    August 29th, 2010 @ 01:15 AM

  6. Jeremy Kemper
  7. Santiago Pastorino
  8. Jeremy Kemper