Merge pull request #30169 from awortham/awortham/awortham/fix-sql-distinct-bug

Ensure sum honors distinct on has_many through

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Ensure `sum` honors `distinct` on `has_many :through` associations
+
+    Fixes #16791
+
+    *Aaron Wortham
+
 *   Fix `COUNT(DISTINCT ...)` with `ORDER BY` and `LIMIT` to keep the existing select list.
 
     *Ryuta Kamizono*

activerecord/lib/active_record/relation/calculations.rb

@@ -239,6 +239,9 @@ module ActiveRecord
           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)

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -1132,6 +1132,32 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     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