diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 77cb167e7c..4323c57b9e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix different `count` calculation when using `size` with manual `select` with DISTINCT. + + Fixes #35214. + + *Juani Villarejo* + + ## Rails 6.0.0.beta2 (February 25, 2019) ## * Fix prepared statements caching to be enabled even when query caching is enabled. diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index bdd3c540bb..4f9ddf302e 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -221,7 +221,6 @@ module ActiveRecord end private - def has_include?(column_name) eager_loading? || (includes_values.present? && column_name && column_name != :all) end @@ -236,10 +235,12 @@ module ActiveRecord if operation == "count" column_name ||= select_for_count if column_name == :all - if distinct && (group_values.any? || select_values.empty? && order_values.empty?) + if !distinct + distinct = distinct_select?(select_for_count) if group_values.empty? + elsif group_values.any? || select_values.empty? && order_values.empty? column_name = primary_key end - elsif column_name.is_a?(::String) && /\bDISTINCT[\s(]/i.match?(column_name) + elsif distinct_select?(column_name) distinct = nil end end @@ -251,6 +252,10 @@ module ActiveRecord end end + def distinct_select?(column_name) + column_name.is_a?(::String) && /\bDISTINCT[\s(]/i.match?(column_name) + end + def aggregate_column(column_name) return column_name if Arel::Expressions === column_name diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 8ac2d55218..f1e35d6ab9 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -473,6 +473,24 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal 6, Account.select("DISTINCT accounts.id").includes(:firm).count end + def test_should_count_manual_select_with_count_all + assert_equal 5, Account.select("DISTINCT accounts.firm_id").count(:all) + end + + def test_should_count_with_manual_distinct_select_and_distinct + assert_equal 4, Account.select("DISTINCT accounts.firm_id").distinct(true).count + end + + def test_should_count_manual_select_with_group_with_count_all + expected = { nil => 1, 1 => 1, 2 => 1, 6 => 2, 9 => 1 } + actual = Account.select("DISTINCT accounts.firm_id").group("accounts.firm_id").count(:all) + assert_equal expected, actual + end + + def test_should_count_manual_with_count_all + assert_equal 6, Account.count(:all) + end + def test_count_selected_arel_attribute assert_equal 5, Account.select(Account.arel_table[:firm_id]).count assert_equal 4, Account.distinct.select(Account.arel_table[:firm_id]).count diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 2de0a81c99..5a3bd1bd4a 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -972,6 +972,13 @@ class RelationTest < ActiveRecord::TestCase assert_queries(1) { assert_equal 11, posts.load.size } end + def test_size_with_eager_loading_and_manual_distinct_select_and_custom_order + accounts = Account.select("DISTINCT accounts.firm_id").order("accounts.firm_id") + + assert_queries(1) { assert_equal 5, accounts.size } + assert_queries(1) { assert_equal 5, accounts.load.size } + end + def test_count_explicit_columns Post.update_all(comments_count: nil) posts = Post.all