Skip to content

Commit 060e09d

Browse files
committed
Fix count(:all) with eager loading and explicit select and order
This follows up ebc09ed. We've still experienced a regression for `size` (`count(:all)`) with eager loading and explicit select and order when upgrading Rails to 5.1. In that case, the eager loading enforces `distinct` to subselect but still keep the custom select, it would cause the ORDER BY with DISTINCT issue. ``` % ARCONN=postgresql bundle exec ruby -w -Itest test/cases/relations_test.rb -n test_size_with_eager_loading_and_custom_select_and_order Using postgresql Run options: -n test_size_with_eager_loading_and_custom_select_and_order --seed 8356 # Running: E Error: RelationTest#test_size_with_eager_loading_and_custom_select_and_order: ActiveRecord::StatementInvalid: PG::InvalidColumnReference: ERROR: for SELECT DISTINCT, ORDER BY expressions must appear in select list LINE 1: ..." ON "comments"."post_id" = "posts"."id" ORDER BY comments.i... ^ ``` As another problem on `distinct` is enforced, the result of `count` becomes fewer than expected if `select` is given explicitly. e.g. ```ruby Post.select(:type).count # => 11 Post.select(:type).distinct.count # => 3 ``` As long as `distinct` is enforced, we need to care to keep the result of `count`. This fixes both the `count` with eager loading problems.
1 parent 39d42db commit 060e09d

File tree

3 files changed

+17
-4
lines changed

3 files changed

+17
-4
lines changed

activerecord/lib/active_record/relation/calculations.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,12 @@ def calculate(operation, column_name)
129129
relation = apply_join_dependency
130130

131131
if operation.to_s.downcase == "count"
132-
relation.distinct!
133-
# PostgreSQL: ORDER BY expressions must appear in SELECT list when using DISTINCT
134-
if (column_name == :all || column_name.nil?) && select_values.empty?
135-
relation.order_values = []
132+
unless distinct_value || distinct_select?(column_name || select_for_count)
133+
relation.distinct!
134+
relation.select_values = [ klass.primary_key || table[Arel.star] ]
136135
end
136+
# PostgreSQL: ORDER BY expressions must appear in SELECT list when using DISTINCT
137+
relation.order_values = []
137138
end
138139

139140
relation.calculate(operation, column_name)

activerecord/test/cases/calculations_test.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,12 @@ def test_count_with_eager_loading_and_custom_order
243243
assert_queries(1) { assert_equal 11, posts.count(:all) }
244244
end
245245

246+
def test_count_with_eager_loading_and_custom_select_and_order
247+
posts = Post.includes(:comments).order("comments.id").select(:type)
248+
assert_queries(1) { assert_equal 11, posts.count }
249+
assert_queries(1) { assert_equal 11, posts.count(:all) }
250+
end
251+
246252
def test_count_with_eager_loading_and_custom_order_and_distinct
247253
posts = Post.includes(:comments).order("comments.id").distinct
248254
assert_queries(1) { assert_equal 11, posts.count }

activerecord/test/cases/relations_test.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,12 @@ def test_size_with_eager_loading_and_custom_order
985985
assert_queries(1) { assert_equal 11, posts.load.size }
986986
end
987987

988+
def test_size_with_eager_loading_and_custom_select_and_order
989+
posts = Post.includes(:comments).order("comments.id").select(:type)
990+
assert_queries(1) { assert_equal 11, posts.size }
991+
assert_queries(1) { assert_equal 11, posts.load.size }
992+
end
993+
988994
def test_size_with_eager_loading_and_custom_order_and_distinct
989995
posts = Post.includes(:comments).order("comments.id").distinct
990996
assert_queries(1) { assert_equal 11, posts.size }

0 commit comments

Comments
 (0)