diff options
| author | Ryuta Kamizono <kamipo@gmail.com> | 2017-10-14 13:19:26 +0900 |
|---|---|---|
| committer | Ryuta Kamizono <kamipo@gmail.com> | 2017-10-14 13:36:51 +0900 |
| commit | 5668dc6b1863ef43be8f8ef0fb1d5db913085fb3 (patch) | |
| tree | 0f46675c24941c36c97426ae6e71829b3f35406d | |
| parent | 3d1ff79742c46930fa35352c42fb585c3408511b (diff) | |
| download | rails-5668dc6b1863ef43be8f8ef0fb1d5db913085fb3.tar.gz rails-5668dc6b1863ef43be8f8ef0fb1d5db913085fb3.tar.bz2 rails-5668dc6b1863ef43be8f8ef0fb1d5db913085fb3.zip | |
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.
| -rw-r--r-- | activerecord/CHANGELOG.md | 6 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/calculations.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/cases/calculations_test.rb | 4 |
3 files changed, 11 insertions, 1 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 288f22a9d3..490908417a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -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 diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 4ef0502893..116bddce85 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -216,7 +216,7 @@ module ActiveRecord 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 diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index b47fd0af41..66bc14b5ab 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -260,6 +260,10 @@ class CalculationsTest < ActiveRecord::TestCase 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] |