This project is archived and is in readonly mode.

#5470 ✓stale
Szymon Nowak

Inconsistent order of columns in SQL WHERE clause in dynamic finders and scopes

Reported by Szymon Nowak | August 27th, 2010 @ 08:17 AM

There's a minor problem with the way SQL is generated.

Venue.find_all_by_city_id_and_category_id(1, 1)
SELECT "venues".* FROM "venues" WHERE ("venues"."category_id" = 1) AND ("venues"."city_id" = 1)

Venue.where(:city_id => 1, :category_id => 1)
SELECT "venues".* FROM "venues" WHERE ("venues"."category_id" = 1) AND ("venues"."city_id" = 1)

Venue.where(:city_id => 1).where(:category_id => 1)
SELECT "venues".* FROM "venues" WHERE ("venues"."city_id" = 1) AND ("venues"."category_id" = 1)

I think it should always keep the order of the columns in the generated SQL, otherwise it becomes tricky to properly setup multicolumn database indexes.

BTW. Not sure if it matters, but I'm using PostgreSQL.

Comments and changes to this ticket

  • Andrew White

    Andrew White August 27th, 2010 @ 09:20 AM

    • Importance changed from “” to “Low”

    If the last example were to change to category_id and city_id I'd consider that a bug - I don't want ActiveRecord reordering the sequence of where that I've applied.

  • Szymon Nowak

    Szymon Nowak August 27th, 2010 @ 09:25 AM

    Me too. I think that the first 2 examples should generate SQL as in the last one.

  • Andrew White

    Andrew White August 27th, 2010 @ 09:48 AM

    The middle one is impossible to fix since you're passing a hash which by definition is unordered. This best that can be done in this case is to sort predictably, which it appears to do alphabetically. The first one is essentially the same as the second one since that is what it called eventually after the name has been split up into the constituent parts.

    You could fix the first one by changing find_by_attributes in ActiveRecord::FinderMethods to chain calls to where rather building a hash and then calling where but there may be a performance impact so that'd need to be checked.

    Why not just create a named scope and exclusively use that?

      scope :find_by_city_then_category, lambda { |city, category| where(:city_id => city).where(:category_id => category) }
    
  • Szymon Nowak

    Szymon Nowak August 27th, 2010 @ 10:12 AM

    It turns out that all methods generate the same query on Ruby 1.9 where hashes are ordered. I'm using 1.8.7 and that's why it generates different queries.

  • Neeraj Singh

    Neeraj Singh August 27th, 2010 @ 02:50 PM

    Instead of

    Venue.find_all_by_city_id_and_category_id(1, 1)
    

    you can also do

    Venue.scoped_by_city_id_and_category_id(1, 1)
    

    The advantage is that scoped_by creates a method after each time it is called. So next time method_missing will not be called and performance will be better. find_by does not do anything like that and would continue to hit method_missing every single time.

    Not directly related to this ticket but just wanted to let people know about it.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:33 PM

    • State changed from “new” to “open”
    • Tag changed from 3.0.0.rc2, activerecord to 300rc2, activerecord

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:33 PM

    • State changed from “open” to “stale”

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="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Pages