diff options
| author | Ernie Miller <ernie@metautonomo.us> | 2010-05-06 16:14:09 -0400 |
|---|---|---|
| committer | Jeremy Kemper <jeremy@bitsweat.net> | 2010-05-06 16:00:39 -0700 |
| commit | 902861a43ae90032063f4a14a3e8b4b9b9c3ca2f (patch) | |
| tree | ca7c3446c4901380f4efd7c21d19cd37ef5f4be6 /activerecord/lib | |
| parent | 6d7f2790cdb6cb23285067ed2bd91fd5122adbbc (diff) | |
| download | rails-902861a43ae90032063f4a14a3e8b4b9b9c3ca2f.tar.gz rails-902861a43ae90032063f4a14a3e8b4b9b9c3ca2f.tar.bz2 rails-902861a43ae90032063f4a14a3e8b4b9b9c3ca2f.zip | |
Fix unintuitive behavior with multiple order and group clauses
[#4545 state:committed]
Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
Diffstat (limited to 'activerecord/lib')
3 files changed, 9 insertions, 8 deletions
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 8ab5eaa724..858d298470 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -195,7 +195,7 @@ module ActiveRecord select_statement << ", #{group_field} AS #{group_alias}" - relation = select(select_statement).group(group) + relation = except(:group).select(select_statement).group(group) calculated_data = @klass.connection.select_all(relation.to_sql) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 7bca12d85e..8d8bb659e1 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -162,13 +162,9 @@ module ActiveRecord arel = arel.take(@limit_value) if @limit_value.present? arel = arel.skip(@offset_value) if @offset_value.present? - @group_values.uniq.each do |g| - arel = arel.group(g) if g.present? - end + arel = arel.group(*@group_values.uniq.select{|g| g.present?}) - @order_values.uniq.each do |o| - arel = arel.order(Arel::SqlLiteral.new(o.to_s)) if o.present? - end + arel = arel.order(*@order_values.uniq.select{|o| o.present?}.map(&:to_s)) selects = @select_values.uniq diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 8fdd64afcc..bb1f138f5b 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -80,10 +80,15 @@ module ActiveRecord options.assert_valid_keys(VALID_FIND_OPTIONS) - [:joins, :select, :group, :having, :order, :limit, :offset, :from, :lock, :readonly].each do |finder| + [:joins, :select, :group, :having, :limit, :offset, :from, :lock, :readonly].each do |finder| relation = relation.send(finder, options[finder]) if options.has_key?(finder) end + # Give precedence to newly-applied orders and groups to play nicely with with_scope + [:group, :order].each do |finder| + relation.send("#{finder}_values=", Array.wrap(options[finder]) + relation.send("#{finder}_values")) if options.has_key?(finder) + end + relation = relation.where(options[:conditions]) if options.has_key?(:conditions) relation = relation.includes(options[:include]) if options.has_key?(:include) relation = relation.extending(options[:extend]) if options.has_key?(:extend) |