Permalink
Browse files

Fix ActiveRecord calculations when grouped by multiple fields

  • Loading branch information...
1 parent c9b2736 commit 8d6eb9b3fb31ad8dd035dced2e699cb00c3c0ba2 @acatighera acatighera committed with tenderlove Nov 16, 2010
Showing with 31 additions and 14 deletions.
  1. +18 −14 activerecord/lib/active_record/relation/calculations.rb
  2. +13 −0 activerecord/test/cases/calculations_test.rb
View
32 activerecord/lib/active_record/relation/calculations.rb
@@ -208,37 +208,41 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
end
def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
- group_attr = @group_values.first
- association = @klass.reflect_on_association(group_attr.to_sym)
- associated = association && association.macro == :belongs_to # only count belongs_to associations
- group_field = associated ? association.primary_key_name : group_attr
- group_alias = column_alias_for(group_field)
- group_column = column_for(group_field)
+ group_attr = @group_values
+ association = @klass.reflect_on_association(group_attr.first.to_sym)
+ associated = group_attr.size == 1 && association && association.macro == :belongs_to # only count belongs_to associations
+ group_fields = Array(associated ? association.primary_key_name : group_attr)
+ group_aliases = []
+ group_columns = {}
+
+ group_fields.each do |field|
+ group_aliases << column_alias_for(field)
+ group_columns[column_alias_for(field)] = column_for(field)
+ end
- group = @klass.connection.adapter_name == 'FrontBase' ? group_alias : group_field
+ group = @klass.connection.adapter_name == 'FrontBase' ? group_aliases : group_fields
if operation == 'count' && column_name == :all
aggregate_alias = 'count_all'
else
aggregate_alias = column_alias_for(operation, column_name)
end
- relation = except(:group).group(group)
- relation.select_values = [
- operation_over_aggregate_column(aggregate_column(column_name), operation, distinct).as(aggregate_alias),
- "#{group_field} AS #{group_alias}"
- ]
+ relation = except(:group).group(group.join(','))
+ relation.select_values = [ operation_over_aggregate_column(aggregate_column(column_name), operation, distinct).as(aggregate_alias) ]
+ group_fields.each_index{ |i| relation.select_values << "#{group_fields[i]} AS #{group_aliases[i]}" }
calculated_data = @klass.connection.select_all(relation.to_sql)
if association
- key_ids = calculated_data.collect { |row| row[group_alias] }
+ key_ids = calculated_data.collect { |row| row[group_aliases.first] }
key_records = association.klass.base_class.find(key_ids)
key_records = Hash[key_records.map { |r| [r.id, r] }]
end
ActiveSupport::OrderedHash[calculated_data.map do |row|
- key = type_cast_calculated_value(row[group_alias], group_column)
+ key = group_aliases.map{|group_alias| type_cast_calculated_value(row[group_alias], group_columns[group_alias])}
+ key = key.first if key.size == 1
key = key_records[key] if associated
[key, type_cast_calculated_value(row[aggregate_alias], column_for(column_name), operation)]
end]
View
13 activerecord/test/cases/calculations_test.rb
@@ -54,6 +54,19 @@ def test_should_group_by_field
c = Account.sum(:credit_limit, :group => :firm_id)
[1,6,2].each { |firm_id| assert c.keys.include?(firm_id) }
end
+
+ def test_should_group_by_multiple_fields
+ c = Account.count(:all, :group => ['firm_id', :credit_limit])
+ [ [nil, 50], [1, 50], [6, 50], [6, 55], [9, 53], [2, 60] ].each { |firm_and_limit| assert c.keys.include?(firm_and_limit) }
+ end
+
+ def test_should_group_by_multiple_fields_having_functions
+ c = Topic.group(:author_name, 'COALESCE(type, title)').count(:all)
+ assert_equal 1, c[["Carl", "The Third Topic of the day"]]
+ assert_equal 1, c[["Mary", "Reply"]]
+ assert_equal 1, c[["David", "The First Topic"]]
+ assert_equal 1, c[["Carl", "Reply"]]
+ end
def test_should_group_by_summed_field
c = Account.sum(:credit_limit, :group => :firm_id)

0 comments on commit 8d6eb9b

Please sign in to comment.