Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix ActiveRecord calculations when grouped by multiple fields

  • Loading branch information...
commit 1681ede605251560c17728d71fe5ed7390dc291d 1 parent 44db47c
@acatighera acatighera authored tenderlove committed
View
31 activerecord/lib/active_record/calculations.rb
@@ -187,7 +187,7 @@ def construct_calculation_sql(operation, column_name, options) #:nodoc:
# A (slower) workaround if we're using a backend, like sqlite, that doesn't support COUNT DISTINCT.
sql = "SELECT COUNT(*) AS #{aggregate_alias}" if use_workaround
- sql << ", #{options[:group_field]} AS #{options[:group_alias]}" if options[:group]
+ options[:group_fields].each_index{|i| sql << ", #{options[:group_fields][i]} AS #{options[:group_aliases][i]}" } if options[:group]
if options[:from]
sql << " FROM #{options[:from]} "
elsif scope && scope[:from] && !use_workaround
@@ -211,8 +211,8 @@ def construct_calculation_sql(operation, column_name, options) #:nodoc:
add_limited_ids_condition!(sql, options, join_dependency) if join_dependency && !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
if options[:group]
- group_key = connection.adapter_name == 'FrontBase' ? :group_alias : :group_field
- sql << " GROUP BY #{options[group_key]} "
+ group_key = connection.adapter_name == 'FrontBase' ? :group_aliases : :group_fields
+ sql << " GROUP BY #{options[group_key].join(',')} "
end
if options[:group] && options[:having]
@@ -239,24 +239,31 @@ def execute_simple_calculation(operation, column_name, column, options) #:nodoc:
end
def execute_grouped_calculation(operation, column_name, column, options) #:nodoc:
- group_attr = options[:group].to_s
- association = 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
- sql = construct_calculation_sql(operation, column_name, options.merge(:group_field => group_field, :group_alias => group_alias))
+ group_attr = options[:group]
+ association = reflect_on_association(group_attr.to_s.to_sym)
+ associated = 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
+
+ sql = construct_calculation_sql(operation, column_name, options.merge(:group_fields => group_fields, :group_aliases => group_aliases))
calculated_data = connection.select_all(sql)
aggregate_alias = column_alias_for(operation, column_name)
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 = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) }
end
calculated_data.inject(ActiveSupport::OrderedHash.new) do |all, 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
value = row[aggregate_alias]
all[key] = type_cast_calculated_value(value, column, operation)
View
13 activerecord/test/cases/calculations_test.rb
@@ -58,6 +58,19 @@ def test_should_group_by_field
[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.count(:all, :group => [:author_name, 'COALESCE(type, title)'])
+ assert_equal 1, c[["Nick", "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)
assert_equal 50, c[1]
Please sign in to comment.
Something went wrong with that request. Please try again.