Fix 500 error sorting merge requests with approvers by milestone
What does this MR do and why?
When adding a Approved By
or Approver
filter and sorting by milestone, a 500
error will occur because of a SQL error:
PG::GroupingError: ERROR: column "milestones.due_date" must appear in
the GROUP BY clause or be used in an aggregate function
This happens because the approval query uses a GROUP BY
query (https://gitlab.com/gitlab-org/gitlab/-/blob/29479455a951d1daf379471a5579c00b60897a43/app/models/concerns/approvable.rb#L24) to
find merge requests that match all approvers. However, this GROUP BY
query breaks the SQL query since it's expected that
milestones.due_date
and milestones.id
are present in that clause.
We can solve this problem in a few ways:
- Add the required grouping columns to this query (this commit).
- Use a CTE to isolate the query for approvers.
- Rewrite the approvers query to avoid the GROUP BY altogether.
The second way appears to work, but there's a risk of running into errors due to nested queries. Plus, it does not appear the CTE improves performance.
The third way may also be possible, but it would possibly require some schema changes or other refactoring.
For now, we can solve this problem by manually adding the required grouping columns when approvers or approvals are specified.
Relates to #223062 (closed)
How to set up and validate locally
- Go to your MR dashboard on
dashboard/merge_requests
. - Go to
Merged
tab. - Add filter
Approved By
with any user selection, by default this can potentially try to load a lot of matches so you can additionally addMilestone
as one more criteria. - Sort by
Milestone Due Date
- Hit Enter/Return to apply the filter.
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
Queries
Before (error)
SELECT "merge_requests".*
FROM "merge_requests"
INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id"
INNER JOIN "approvals" ON "approvals"."merge_request_id" = "merge_requests"."id"
INNER JOIN "users" ON "users"."id" = "approvals"."user_id"
LEFT JOIN project_features ON projects.id = project_features.project_id
LEFT OUTER JOIN milestones ON merge_requests.milestone_id = milestones.id
WHERE (EXISTS
(SELECT 1
FROM "project_authorizations"
WHERE "project_authorizations"."user_id" = 64248
AND (project_authorizations.project_id = projects.id))
OR projects.visibility_level IN (0,
10,
20))
AND ("project_features"."merge_requests_access_level" > 0
OR "project_features"."merge_requests_access_level" IS NULL)
AND ("merge_requests"."state_id" IN (3))
AND "projects"."archived" = FALSE
AND "users"."username" = 'stanhu'
GROUP BY "merge_requests"."id",
HAVING (COUNT(users.id) = 1)
ORDER BY milestones.due_date IS NULL,
milestones.id IS NULL,
milestones.due_date ASC,
"merge_requests"."id" DESC
LIMIT 20
OFFSET 0
After
SELECT "merge_requests".*
FROM "merge_requests"
INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id"
INNER JOIN "approvals" ON "approvals"."merge_request_id" = "merge_requests"."id"
INNER JOIN "users" ON "users"."id" = "approvals"."user_id"
LEFT JOIN project_features ON projects.id = project_features.project_id
LEFT OUTER JOIN milestones ON merge_requests.milestone_id = milestones.id
WHERE (EXISTS
(SELECT 1
FROM "project_authorizations"
WHERE "project_authorizations"."user_id" = 64428
AND (project_authorizations.project_id = projects.id))
OR projects.visibility_level IN (0,
10,
20))
AND ("project_features"."merge_requests_access_level" > 0
OR "project_features"."merge_requests_access_level" IS NULL)
AND ("merge_requests"."state_id" IN (3))
AND "projects"."archived" = FALSE
AND "users"."username" = 'stanhu'
GROUP BY "merge_requests"."id",
"merge_requests"."id",
"milestones"."id",
"milestones"."due_date"
HAVING (COUNT(users.id) = 1)
ORDER BY milestones.due_date IS NULL,
milestones.id IS NULL,
milestones.due_date ASC,
"merge_requests"."id" DESC
LIMIT 20
OFFSET 0
Query plan (warm cache): https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/12077/commands/42853
Time: 139.380 ms
- planning: 3.646 ms
- execution: 135.734 ms
- I/O read: 0.000 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 94367 (~737.20 MiB) from the buffer pool
- reads: 0 from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0
-
I have evaluated the MR acceptance checklist for this MR.