aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorRafael Sales <rafaelcds@gmail.com>2015-10-12 23:18:51 -0300
committerRafael Sales <rafaelcds@gmail.com>2015-10-22 04:22:57 -0300
commitc2d33c4abf01ec25b2d1528a7dae923aef5a30af (patch)
tree309b3035ad66d0e62467ceeecfd0d9d3061df10a /activerecord
parentd8b076c90efac796e8664c6eada8d08afac4bea3 (diff)
downloadrails-c2d33c4abf01ec25b2d1528a7dae923aef5a30af.tar.gz
rails-c2d33c4abf01ec25b2d1528a7dae923aef5a30af.tar.bz2
rails-c2d33c4abf01ec25b2d1528a7dae923aef5a30af.zip
Fix generated projection fields in group by query
Closes #21922 Let `Book(id, author_id)`, `Photo(id, book_id, author_id)` and `Author(id)` Running `Book.group(:author_id).joins(:photos).count` will produce: * Rails 4.2 - conflicts `author_id` in both projection and group by: ```sql SELECT COUNT(*) AS count_all, author_id AS author_id FROM "books" INNER JOIN "photos" ON "photos"."book_id" = "books"."id" GROUP BY author_id ``` * Master (9d02a25) - conflicts `author_id` only in projection: ```sql SELECT COUNT(*) AS count_all, author_id AS author_id FROM "books" INNER JOIN "photos" ON "photos"."book_id" = "books"."id" GROUP BY "books"."author_id" ``` * With this fix: ```sql SELECT COUNT(*) AS count_all, "books"."author_id" AS books_author_id FROM "books" INNER JOIN "photos" ON "photos"."book_id" = "books"."id" GROUP BY "books"."author_id" ```
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md7
-rw-r--r--activerecord/lib/active_record/relation/calculations.rb3
-rw-r--r--activerecord/test/cases/calculations_test.rb5
3 files changed, 14 insertions, 1 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..6d3e5327fb 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -275,6 +275,7 @@ module ActiveRecord
else
group_fields = group_attrs
end
+ group_fields = arel_columns(group_fields)
group_aliases = group_fields.map { |field|
column_alias_for(field)
@@ -299,7 +300,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_fields.zip(group_aliases).map { |field,aliaz|
if field.respond_to?(:as)
field.as(aliaz)
else
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