Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Active Record distinct & order #count regression #32005

Merged
merged 1 commit into from
Feb 27, 2018
Merged

Active Record distinct & order #count regression #32005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation/calculations.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def calculate(operation, column_name)
if has_include?(column_name)
relation = apply_join_dependency

if operation.to_s.downcase == "count" && !distinct_value
if operation.to_s.downcase == "count"
relation.distinct!
# PostgreSQL: ORDER BY expressions must appear in SELECT list when using DISTINCT
if (column_name == :all || column_name.nil?) && select_values.empty?
Expand Down
8 changes: 8 additions & 0 deletions activerecord/test/cases/calculations_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,14 @@ def test_count_with_eager_loading_and_custom_order
assert_queries(1) { assert_equal 11, posts.count(:all) }
end

def test_count_with_eager_loading_and_custom_order_and_distinct
posts = Post.includes(:comments).order("comments.id").distinct
assert_queries(1) { assert_equal 11, posts.count }
assert_queries(1) { assert_equal 11, posts.count(:all) }
assert_queries(1) { assert_equal 11, posts.size }
assert_queries(1) { assert_equal 11, posts.load.size }
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) }
Expand Down