mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix GROUP BY queries to apply LIMIT/OFFSET after aggregations
If `eager_loading` is true, `apply_join_dependency` force applies LIMIT/OFFSET before JOINs by `limited_ids_for` to keep parent records count. But for aggregation queries, LIMIT/OFFSET should be applied after aggregations the same as SQL semantics. And also, we could not replace SELECT list by `limited_ids_for` when a query has a GROUP BY clause. It had never been worked since it will causes generating invalid SQL for MySQL, PostgreSQL, and probably most backends. ``` % ARCONN=postgresql be ruby -w -Itest test/cases/calculations_test.rb -n test_group_by_with_limit Using postgresql Run options: -n test_group_by_with_limit --seed 20925 # Running: E Error: CalculationsTest#test_group_by_with_limit: ActiveRecord::StatementInvalid: PG::GroupingError: ERROR: column "posts.id" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT DISTINCT "posts"."id", "posts"."type" AS alias_0 FRO... ^ : SELECT DISTINCT "posts"."id", "posts"."type" AS alias_0 FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" GROUP BY "posts"."type" ORDER BY "posts"."type" ASC LIMIT $1 ``` Fixes #8103. Closes #27249.
This commit is contained in:
parent
5dc72378b7
commit
63e35a1323
2 changed files with 19 additions and 1 deletions
|
@ -379,7 +379,7 @@ module ActiveRecord
|
|||
)
|
||||
end
|
||||
|
||||
def apply_join_dependency(eager_loading: true)
|
||||
def apply_join_dependency(eager_loading: group_values.empty?)
|
||||
join_dependency = construct_join_dependency
|
||||
relation = except(:includes, :eager_load, :preload).joins!(join_dependency)
|
||||
|
||||
|
|
|
@ -705,6 +705,24 @@ class CalculationsTest < ActiveRecord::TestCase
|
|||
assert_equal [], Topic.includes(:replies).order(:id).offset(5).pluck(:id)
|
||||
end
|
||||
|
||||
def test_group_by_with_limit
|
||||
expected = { "Post" => 8, "SpecialPost" => 1 }
|
||||
actual = Post.includes(:comments).group(:type).order(:type).limit(2).count("comments.id")
|
||||
assert_equal expected, actual
|
||||
end
|
||||
|
||||
def test_group_by_with_offset
|
||||
expected = { "SpecialPost" => 1, "StiPost" => 2 }
|
||||
actual = Post.includes(:comments).group(:type).order(:type).offset(1).count("comments.id")
|
||||
assert_equal expected, actual
|
||||
end
|
||||
|
||||
def test_group_by_with_limit_and_offset
|
||||
expected = { "SpecialPost" => 1 }
|
||||
actual = Post.includes(:comments).group(:type).order(:type).offset(1).limit(1).count("comments.id")
|
||||
assert_equal expected, actual
|
||||
end
|
||||
|
||||
def test_pluck_not_auto_table_name_prefix_if_column_included
|
||||
Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
|
||||
ids = Company.includes(:contracts).pluck(:developer_id)
|
||||
|
|
Loading…
Reference in a new issue