diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c1c511a65cdeafff0494d116c5240aaaa706ae20..03609a7a059a89dcecf82a78060e1e0565f7c7da 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Ensure `sum` honors `distinct` on `has_many :through` associations + + Fixes #16791 + + *Aaron Wortham + * When using `Relation#or`, extract the common conditions and put them before the OR condition. *Maxime Handfield Lapointe* diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index d3b5be6bced1780a695e35e6f9677bb866ced71b..42d43224fa31ba2da3d8c968fda39795aa6cf5fd 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -259,6 +259,9 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc: column = aggregate_column(column_name) select_value = operation_over_aggregate_column(column, operation, distinct) + if operation == "sum" && distinct + select_value.distinct = true + end column_alias = select_value.alias column_alias ||= @klass.connection.column_name_for_operation(operation, select_value) diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 4c2723addc0f5df86a10d72297ba0e9783b967f9..f4e907ba90a33af2b045dff9fc371151b2f7a613 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1139,6 +1139,32 @@ def test_has_many_through_obeys_order_on_through_association assert_equal ["parrot", "bulbul"], owner.toys.map { |r| r.pet.name } end + def test_has_many_through_associations_sum_on_columns + post1 = Post.create(title: "active", body: "sample") + post2 = Post.create(title: "inactive", body: "sample") + + person1 = Person.create(first_name: "aaron", followers_count: 1) + person2 = Person.create(first_name: "schmit", followers_count: 2) + person3 = Person.create(first_name: "bill", followers_count: 3) + person4 = Person.create(first_name: "cal", followers_count: 4) + + Reader.create(post_id: post1.id, person_id: person1.id) + Reader.create(post_id: post1.id, person_id: person2.id) + Reader.create(post_id: post1.id, person_id: person3.id) + Reader.create(post_id: post1.id, person_id: person4.id) + + Reader.create(post_id: post2.id, person_id: person1.id) + Reader.create(post_id: post2.id, person_id: person2.id) + Reader.create(post_id: post2.id, person_id: person3.id) + Reader.create(post_id: post2.id, person_id: person4.id) + + active_persons = Person.joins(:readers).joins(:posts).distinct(true).where("posts.title" => "active") + + assert_equal active_persons.map(&:followers_count).reduce(:+), 10 + assert_equal active_persons.sum(:followers_count), 10 + assert_equal active_persons.sum(:followers_count), active_persons.map(&:followers_count).reduce(:+) + end + def test_has_many_through_associations_on_new_records_use_null_relations person = Person.new