This project is archived and is in readonly mode.
build_arel causes counterintuitive behavior with group and order
Reported by Ernie Miller | May 6th, 2010 @ 09:28 PM
Current version of Arel does not handle multiple calls to order and group as it would appear build_arel expects it to.
ruby-1.8.7-p249 > Article.order(:id, :created_by).to_sql
=> "SELECT \"articles\".* FROM \"articles\" ORDER BY created_by, id"
ruby-1.8.7-p249 > Article.group(:id, :created_by).to_sql
=> "SELECT \"articles\".* FROM \"articles\" GROUP BY created_by"
With order, the last added order is given precedence. With group_by, only one call is allowed. This patch calls both a single time with the entire usable list of order/group parameters and results in the expected behavior.
Thanks!
Comments and changes to this ticket
-
Ernie Miller May 6th, 2010 @ 09:34 PM
- Assigned user set to Jeremy Kemper
Jeremy,
As much as I hate "assigning" tickets since you and I talked about this in IRC and you described the desired behavior, I figure you're the guy to send it to. :)
-
Jeremy Kemper May 6th, 2010 @ 10:53 PM
- State changed from new to open
1) Error: test_find_keeps_multiple_group_values(BasicsTest): ActiveRecord::StatementInvalid: PGError: ERROR: column "developers.id" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT "developers".* FROM "developers" GROUP BY ... ^ : SELECT "developers".* FROM "developers" GROUP BY developers.name, developers.salary
-
Jeremy Kemper May 7th, 2010 @ 12:06 AM
Thanks! Squashed and removed the extra commit. Looks like a good change but it needs its own test.
-
Repository May 7th, 2010 @ 12:06 AM
- State changed from open to committed
(from [902861a43ae90032063f4a14a3e8b4b9b9c3ca2f]) Fix unintuitive behavior with multiple order and group clauses
[#4545 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/902861a43ae90032063f4a14a3e8b4... -
Ernie Miller May 7th, 2010 @ 01:36 AM
Works for me! I only added the last patch in because I inadvertently removed the SqlLiteral in the first one -- see the line:
- arel = arel.order(Arel::SqlLiteral.new(o.to_s)) if o.present?
But still, it passes all tests either way, and I think in the case of order clauses the SqlLiteral is redundant if you're already doing a to_s:
articles = Article.arel_table articles.order(Arel::SqlLiteral.new('blah')).orders == articles.order('blah').orders => true
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>
People watching this ticket
Attachments
Referenced by
- 4545 build_arel causes counterintuitive behavior with group and order [#4545 state:committed]