This project is archived and is in readonly mode.

#6290 open

[PATCH] except doesn't work in different scopes

Reported by 2kan | January 15th, 2011 @ 12:49 AM | in 3.1

Here is an example:

class A < ActiveRecord::Base
  scope :order_by_salary, order("salary DESC")
  scope :reorder_by_name, except(:order).order("name DESC")

So this code won't work:


Comments and changes to this ticket

  • 2kan

    2kan January 15th, 2011 @ 12:52 AM

    Here is my patch (with tests) for it.

  • 2kan

    2kan January 15th, 2011 @ 12:55 AM

    • Assigned user changed from “Santiago Pastorino” to “Aaron Patterson”
    • Tag changed from activerecord scopes, rails edge, scope to activerecord scopes, rails edge, bug, patch, scope
  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 06:30 PM

    • State changed from “new” to “open”
    • Milestone set to 3.1
    • Importance changed from “” to “Low”

    The expression scope :reorder_by_name, except(:order).order("name DESC") is computed at evaluation time.

    You need to do this ...

    class A < ActiveRecord::Base
      scope :order_by_salary, order("salary DESC")
      scope :reorder_by_name, lambda { except(:order).order("name DESC") }

    Anyways seems the code I showed you is not currently working on master with named scopes even though it works if I do ...

    >> Post.order(:title).to_sql
    => "SELECT \"posts\".* FROM \"posts\"  ORDER BY title"
    >> Post.order(:title).except(:order).to_sql
    => "SELECT \"posts\".* FROM \"posts\" "
    >> Post.order(:title).except(:order).order(:body).to_sql
    => "SELECT \"posts\".* FROM \"posts\"  ORDER BY body"

    So we need to fix the issue for named scopes.

  • Adam Wróbel

    Adam Wróbel February 2nd, 2011 @ 11:27 PM

    I suggest moving order from MULTI_VALUE_METHODS to SINGLE_VALUE_METHODS and overwriting the order array with each merged scope or new call to order(). There's no benefit in using:




    And the fact that both of the below calls return Posts by [:created_at,:title] order is just plain misleading:

    class Post < ActiveRecord::Base
      default_scope order(:created_at)
      scope :by_title, order(:title)
  • 2kan

    2kan February 2nd, 2011 @ 11:50 PM

    Adam Wróbel, order is a multi value and I can give you a lot of examples from my real apps where you need to use queries like:

    SELECT * from employers WHERE ORDER BY salary ASC, name ASC

    Here you sort employers by salary and if salary is equal you sort then by name. It is a common task. And so if we're allow user to write order('salary ASC, name ASC') I think we should allow to write order(:salary).order(:name).

  • 2kan

    2kan February 2nd, 2011 @ 11:56 PM

    Adam Wróbel, i mean that it is not intuitive why we can do:


    and can't:


    or do something like:

    class Post < ActiveRecord::Base
     scope popular, where('rating > 1').order('rating DESC')
     scope recent, order('created_at DESC')
  • 2kan

    2kan February 2nd, 2011 @ 11:58 PM

    Oh, sorry, it is 2:57 AM here. In my previous comment we can't do first and can second and so can't do the last example with a Post class.

  • Adam Wróbel

    Adam Wróbel February 3rd, 2011 @ 12:05 AM

    An alternative would be to overwrite on merge, but append on order(). That would be similar to where(:column => value). Multiple where() calls create OR but during a merge only the last one is left.

  • Adam Wróbel

    Adam Wróbel February 3rd, 2011 @ 12:22 AM

    I know you specify multiple columns for order, but when I referred to multi and single values I meant values that get merged and values that get overwritten. My suggestion was to overwrite array of order columns from the current scope with array of order columns from the relation being merged in.

    The Post.popular.recent example makes sense and might be a reason for why none of my suggestions should be used, but then again order is such a simple operation that it's easy to list all the columns you need in one place. Usually I'd just be confused that my last call to order wasn't the one used rather than happy that I was was able to merge two named order scopes. Take this test from rails tests (tests/cases/relations_test.rb):

    def test_order_using_scoping
      car1 = CoolCar.order('id DESC').scoping do
        CoolCar.find(:first, :order => 'id asc')
      assert_equal 'zyke',
      car2 = FastCar.order('id DESC').scoping do
        FastCar.find(:first, :order => 'id asc')
      assert_equal 'zyke',

    To me it reads "def test_we_have_a_bug". I mean - I've specified the order I want right there - in the call to find() - why do you do it reverse, rails?

  • 2kan

    2kan February 3rd, 2011 @ 04:34 PM

    Santiago Pastorino, Yes this code computes at evaluation time, but it doesn't matter. I think that there is no reason (point me if i'm wrong) to allow user to use except only inside lambdas. I understand that I need to use lambda when I use or something but here I need some additional knowledge about how except, order, where and etc works inside rails. I don't think that it is good. I've added to my patch test with scope with lambda like:

    lambda { except(:order).order("name DESC") }

    and test passes.

    How it works and why I think we can calculate except at evaluation time. A've added a flag into the Relation that it is except and store what we want to except, so when we merge relations when calling the scopes we can figure out that we need to except some values.

    Please, point me if I made a mistake somewhere.

  • 2kan

    2kan March 21st, 2011 @ 01:44 PM

    Oops, I've found that my last uploaded patch wasn't good enough (I've uploaded the wrong diff, sorry). Here is the correct one.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href=""></a>

Referenced by