diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 8ab5eaa724cc91b60469913b1192a5b1caba2454..858d2984707617d93d809c2cebfd9dee2e39d6dd 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -195,7 +195,7 @@ def execute_grouped_calculation(operation, column_name) #:nodoc: 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 7bca12d85e6fd18c396c6159b08e940b624f5e0b..8d8bb659e18c56fa93d880d1de7ce05e5299bf77 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -162,13 +162,9 @@ def build_arel 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 8fdd64afcc6432b7aa2b2a52aef62bd3f30ea3cb..bb1f138f5b33a3657db0974f234a65c61a1b274c 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -80,10 +80,15 @@ def apply_finder_options(options) 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) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 3623680de917ed2d14d7c84ea33f0e38930cf95f..bbc4e543d5a685fd1e334b5c53cd7646cc247483 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1994,6 +1994,16 @@ def test_find_multiple_ordered_last assert_equal last, Developer.find(:all, :order => 'developers.name, developers.salary DESC').last end + def test_find_keeps_multiple_order_values + combined = Developer.find(:all, :order => 'developers.name, developers.salary') + assert_equal combined, Developer.find(:all, :order => ['developers.name', 'developers.salary']) + end + + def test_find_keeps_multiple_group_values + combined = Developer.find(:all, :group => 'developers.name, developers.salary, developers.id, developers.created_at, developers.updated_at') + assert_equal combined, Developer.find(:all, :group => ['developers.name', 'developers.salary', 'developers.id', 'developers.created_at', 'developers.updated_at']) + end + def test_find_symbol_ordered_last last = Developer.find :last, :order => :salary assert_equal last, Developer.find(:all, :order => :salary).last