diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/calculations.rb | 18 | ||||
| -rw-r--r-- | activerecord/test/cases/calculations_test.rb | 6 | ||||
| -rw-r--r-- | activerecord/test/cases/relations_test.rb | 6 |
4 files changed, 24 insertions, 10 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 73970c99d0..d5cce3f47c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix `count(:all)` to correctly work `distinct` with custom SELECT list. + + *Ryuta Kamizono* + * Using subselect for `delete_all` with `limit` or `offset`. *Ryuta Kamizono* diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index d49472fc70..cb0b06cfdc 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -217,7 +217,7 @@ module ActiveRecord if operation == "count" column_name ||= select_for_count if column_name == :all - if distinct && (group_values.any? || !(has_limit_or_offset? && order_values.any?)) + if distinct && (group_values.any? || select_values.empty? && order_values.empty?) column_name = primary_key end elsif column_name =~ /\s*DISTINCT[\s(]+/i @@ -249,7 +249,7 @@ module ActiveRecord def execute_simple_calculation(operation, column_name, distinct) #:nodoc: column_alias = column_name - if operation == "count" && has_limit_or_offset? + if operation == "count" && (column_name == :all && distinct || has_limit_or_offset?) # Shortcut when limit is zero. return 0 if limit_value == 0 @@ -391,14 +391,12 @@ module ActiveRecord end def build_count_subquery(relation, column_name, distinct) - relation.select_values = [ - if column_name == :all - distinct ? table[Arel.star] : Arel.sql(FinderMethods::ONE_AS_ONE) - else - column_alias = Arel.sql("count_column") - aggregate_column(column_name).as(column_alias) - end - ] + if column_name == :all + relation.select_values = [ Arel.sql(FinderMethods::ONE_AS_ONE) ] unless distinct + else + column_alias = Arel.sql("count_column") + relation.select_values = [ aggregate_column(column_name).as(column_alias) ] + end subquery = relation.arel.as(Arel.sql("subquery_for_count")) select_value = operation_over_aggregate_column(column_alias || Arel.star, "count", false) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 55b50e4f84..5eda39a0c7 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -236,6 +236,12 @@ class CalculationsTest < ActiveRecord::TestCase end end + def test_distinct_count_all_with_custom_select_and_order + accounts = Account.distinct.select("credit_limit % 10").order(Arel.sql("credit_limit % 10")) + assert_queries(1) { assert_equal 3, accounts.count(:all) } + assert_queries(1) { assert_equal 3, accounts.load.size } + end + def test_distinct_count_with_order_and_limit assert_equal 4, Account.distinct.order(:firm_id).limit(4).count end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 474f8c3da7..7785f8c99b 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -963,6 +963,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal 11, posts.distinct(false).select(:comments_count).count end + def test_size_with_distinct + posts = Post.distinct.select(:author_id, :comments_count) + assert_queries(1) { assert_equal 8, posts.size } + assert_queries(1) { assert_equal 8, posts.load.size } + end + def test_update_all_with_scope tag = Tag.first Post.tagged_with(tag.id).update_all title: "rofl" |