Permalink
Browse files

Fix `COUNT(DISTINCT ...)` for `GROUP BY` with `ORDER BY` and `LIMIT`

This is the fix for the regression of #29848.

In #29848, I've kept existing select list in the subquery for the count
if ORDER BY is given. But it had accidentally affect to GROUP BY
queries also. It should keep the previous behavior in that case.

Fixes #30886.
  • Loading branch information...
kamipo committed Oct 14, 2017
1 parent 3d1ff79 commit 5668dc6b1863ef43be8f8ef0fb1d5db913085fb3
@@ -1,3 +1,9 @@
* Fix `COUNT(DISTINCT ...)` for `GROUP BY` with `ORDER BY` and `LIMIT`.
Fixes #30886.
*Ryuta Kamizono*
* PostgreSQL `tsrange` now preserves subsecond precision.
PostgreSQL 9.1+ introduced range types, and Rails added support for using
@@ -216,7 +216,7 @@ def perform_calculation(operation, column_name)
if operation == "count"
column_name ||= select_for_count
if column_name == :all
if distinct && !(has_limit_or_offset? && order_values.any?)
if distinct && (group_values.any? || !(has_limit_or_offset? && order_values.any?))
column_name = primary_key
end
elsif column_name =~ /\s*DISTINCT[\s(]+/i
@@ -260,6 +260,10 @@ def test_distinct_joins_count_with_order_and_limit_and_offset
assert_equal 3, Account.joins(:firm).distinct.order(:firm_id).limit(3).offset(2).count
end
def test_distinct_count_with_group_by_and_order_and_limit
assert_equal({ 6 => 2 }, Account.group(:firm_id).distinct.order("1 DESC").limit(1).count)
end
def test_should_group_by_summed_field_having_condition
c = Account.group(:firm_id).having("sum(credit_limit) > 50").sum(:credit_limit)
assert_nil c[1]

0 comments on commit 5668dc6

Please sign in to comment.