Skip to content

Commit d38e5d2

Browse files
kamipojeremy
authored andcommitted
Suppress DISTINCT clause outside aggregate function
`DISTINCT` clause is applied inside aggregate function by `operation_over_aggregate_column` if needed. Unneeded outside aggregate function. ```ruby # Before author.unique_categorized_posts.count # => SELECT DISTINCT COUNT(DISTINCT "posts"."id") FROM "posts" INNER JOIN "categorizations" ON "posts"."id" = "categorizations"."post_id" WHERE "categorizations"."author_id" = ? [["author_id", 2]] # After author.unique_categorized_posts.count # => SELECT COUNT(DISTINCT "posts"."id") FROM "posts" INNER JOIN "categorizations" ON "posts"."id" = "categorizations"."post_id" WHERE "categorizations"."author_id" = ? [["author_id", 2]] ``` Closes rails#27615
1 parent 558336e commit d38e5d2

File tree

2 files changed

+13
-2
lines changed

2 files changed

+13
-2
lines changed

activerecord/lib/active_record/relation/calculations.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
232232
query_builder = build_count_subquery(spawn, column_name, distinct)
233233
else
234234
# PostgreSQL doesn't like ORDER BY when there are no GROUP BY
235-
relation = unscope(:order)
235+
relation = unscope(:order).distinct!(false)
236236

237237
column = aggregate_column(column_name)
238238

@@ -292,7 +292,7 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
292292
end
293293
}
294294

295-
relation = except(:group)
295+
relation = except(:group).distinct!(false)
296296
relation.group_values = group_fields
297297
relation.select_values = select_values
298298

activerecord/test/cases/calculations_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,17 @@ def test_count_on_invalid_columns_raises
227227
assert_match "credit_limit, firm_name", e.message
228228
end
229229

230+
def test_apply_distinct_in_count
231+
queries = assert_sql do
232+
Account.distinct.count
233+
Account.group(:firm_id).distinct.count
234+
end
235+
236+
queries.each do |query|
237+
assert_match %r{\ASELECT(?! DISTINCT) COUNT\(DISTINCT\b}, query
238+
end
239+
end
240+
230241
def test_should_group_by_summed_field_having_condition
231242
c = Account.group(:firm_id).having("sum(credit_limit) > 50").sum(:credit_limit)
232243
assert_nil c[1]

0 commit comments

Comments
 (0)