Fix 500 errors when sorting by merged or closed date with approvers
What does this MR do and why?
When adding a Approved By
or Approver
filter and sorting by merged
date, a 500 error will occur because of a SQL error:
PG::GroupingError: ERROR: column "merge_request_metrics.id" 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 find
merge requests that match all approvers (https://gitlab.com/gitlab-org/gitlab/-/blob/29479455a951d1daf379471a5579c00b60897a43/app/models/concerns/approvable.rb#L24). However, this GROUP BY
query breaks the SQL query since it's expected that
merge_request_metrics.id
is present in that clause.
This is similar to a bug fixed with sorting by milestones: !97726 (merged).
The UI also sorts by merged_at
, so we need to account for that too.
Relates to #375192 (closed)
How to set up and validate locally
- Go to the
/dashboard/merge_requests
. - Select a filter with
Approver =
. - On the sort order on the right, click
Merged date
.
SQL queries
Working query
SELECT "merge_requests".*, "merge_request_metrics"."merged_at" AS merge_request_metrics_merged_at, "merge_request_metrics"."id" AS merge_request_metrics_id FROM "merge_requests" INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" INNER JOIN "merge_request_metrics" ON "merge_request_metrics"."merge_request_id" = "merge_requests"."id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 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 (1)) AND "projects"."archived" = FALSE AND "merge_requests"."id" IN (SELECT "merge_requests"."id" FROM ( (SELECT "merge_requests".* FROM "merge_requests" INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" INNER JOIN "approval_merge_request_rules" ON "approval_merge_request_rules"."merge_request_id" = "merge_requests"."id" INNER JOIN "approval_merge_request_rules_users" ON "approval_merge_request_rules_users"."approval_merge_request_rule_id" = "approval_merge_request_rules"."id" INNER JOIN "users" ON "users"."id" = "approval_merge_request_rules_users"."user_id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 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 (1)) AND "projects"."archived" = FALSE AND "users"."username" = 'root' GROUP BY "merge_requests"."id" HAVING (COUNT(users.id) = 1)) UNION (SELECT "merge_requests".* FROM "merge_requests" INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" INNER JOIN "approval_merge_request_rules" ON "approval_merge_request_rules"."merge_request_id" = "merge_requests"."id" INNER JOIN "approval_merge_request_rules_groups" ON "approval_merge_request_rules_groups"."approval_merge_request_rule_id" = "approval_merge_request_rules"."id" INNER JOIN "namespaces" ON "namespaces"."id" = "approval_merge_request_rules_groups"."group_id" AND "namespaces"."type" = 'Group' INNER JOIN "members" ON "members"."source_type" = 'Namespace' AND "members"."requested_at" IS NULL AND "members"."access_level" != 5 AND "members"."source_id" = "namespaces"."id" AND "members"."type" = 'GroupMember' INNER JOIN "users" ON "users"."id" = "members"."user_id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 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 (1)) AND "projects"."archived" = FALSE AND "users"."username" = 'root' GROUP BY "merge_requests"."id" HAVING (COUNT(users.id) = 1)) UNION (SELECT "merge_requests".* FROM "merge_requests" INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" INNER JOIN "approval_project_rules" ON "approval_project_rules"."project_id" = "projects"."id" INNER JOIN "approval_project_rules_users" ON "approval_project_rules_users"."approval_project_rule_id" = "approval_project_rules"."id" INNER JOIN "users" ON "users"."id" = "approval_project_rules_users"."user_id" LEFT OUTER JOIN "approval_merge_request_rules" ON "approval_merge_request_rules"."merge_request_id" = "merge_requests"."id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 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 (1)) AND "projects"."archived" = FALSE AND "approval_merge_request_rules"."id" IS NULL AND "users"."username" = 'root' GROUP BY "merge_requests"."id" HAVING (COUNT(users.id) = 1)) UNION (SELECT "merge_requests".* FROM "merge_requests" INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" INNER JOIN "approval_project_rules" ON "approval_project_rules"."project_id" = "projects"."id" INNER JOIN "approval_project_rules_groups" ON "approval_project_rules_groups"."approval_project_rule_id" = "approval_project_rules"."id" INNER JOIN "namespaces" ON "namespaces"."id" = "approval_project_rules_groups"."group_id" AND "namespaces"."type" = 'Group' INNER JOIN "members" ON "members"."source_type" = 'Namespace' AND "members"."requested_at" IS NULL AND "members"."access_level" != 5 AND "members"."source_id" = "namespaces"."id" AND "members"."type" = 'GroupMember' INNER JOIN "users" ON "users"."id" = "members"."user_id" LEFT OUTER JOIN "approval_merge_request_rules" ON "approval_merge_request_rules"."merge_request_id" = "merge_requests"."id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 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 (1)) AND "projects"."archived" = FALSE AND "approval_merge_request_rules"."id" IS NULL AND "users"."username" = 'root' GROUP BY "merge_requests"."id" HAVING (COUNT(users.id) = 1))) merge_requests INNER JOIN "projects" ON "projects"."id" = "merge_requests"."target_project_id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 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 (1)) AND "projects"."archived" = FALSE) AND "merge_requests"."target_project_id" = "merge_request_metrics"."target_project_id" GROUP BY "merge_requests"."id", "merge_request_metrics"."id", "merge_request_metrics"."merged_at" ORDER BY "merge_request_metrics"."merged_at" ASC NULLS LAST, "merge_request_metrics"."id" DESC LIMIT 20 OFFSET 0
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.