diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 7 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/calculations.rb | 15 | ||||
| -rw-r--r-- | activerecord/test/cases/calculations_test.rb | 5 | ||||
| -rw-r--r-- | activerecord/test/models/company.rb | 1 |
4 files changed, 17 insertions, 11 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 258b2be899..e1f543c3a1 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Queries such as `Computer.joins(:monitor).group(:status).count` will now be + interpreted as `Computer.joins(:monitor).group('computers.status').count` + so that when `Computer` and `Monitor` have both `status` columns we don't + have conflicts in projection. + + *Rafael Sales* + * Add ability to default to `uuid` as primary key when generating database migrations Set `Rails.application.config.active_record.primary_key = :uuid` diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 8b9367acc7..f45844a9ea 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -275,15 +275,10 @@ module ActiveRecord else group_fields = group_attrs end + group_fields = arel_columns(group_fields) - group_aliases = group_fields.map { |field| - column_alias_for(field) - } - group_columns = group_aliases.zip(group_fields).map { |aliaz,field| - [aliaz, field] - } - - group = group_fields + group_aliases = group_fields.map { |field| column_alias_for(field) } + group_columns = group_aliases.zip(group_fields) if operation == 'count' && column_name == :all aggregate_alias = 'count_all' @@ -299,7 +294,7 @@ module ActiveRecord ] select_values += select_values unless having_clause.empty? - select_values.concat arel_columns(group_fields).zip(group_aliases).map { |field,aliaz| + select_values.concat group_columns.map { |aliaz, field| if field.respond_to?(:as) field.as(aliaz) else @@ -308,7 +303,7 @@ module ActiveRecord } relation = except(:group) - relation.group_values = group + relation.group_values = group_fields relation.select_values = select_values calculated_data = @klass.connection.select_all(relation, nil, relation.bound_attributes) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index a0e4b4971b..4a0e6f497f 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -455,6 +455,11 @@ class CalculationsTest < ActiveRecord::TestCase [1,6,2,9].each { |firm_id| assert c.keys.include?(firm_id) } end + def test_should_count_field_of_root_table_with_conflicting_group_by_column + assert_equal({ 1 => 1 }, Firm.joins(:accounts).group(:firm_id).count) + assert_equal({ 1 => 1 }, Firm.joins(:accounts).group('accounts.firm_id').count) + end + def test_count_with_no_parameters_isnt_deprecated assert_not_deprecated { Account.count } end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index a96b8ef0f2..1dcd9fc21e 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -10,7 +10,6 @@ class Company < AbstractCompany has_one :dummy_account, :foreign_key => "firm_id", :class_name => "Account" has_many :contracts has_many :developers, :through => :contracts - has_many :accounts scope :of_first_firm, lambda { joins(:account => :firm). |