Skip to content

Fix 500 errors when sorting by merged or closed date with approvers

Stan Hu requested to merge sh-fix-sort-by-merged-at-and-approvers into master

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

  1. Go to the /dashboard/merge_requests.
  2. Select a filter with Approver =.
  3. 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.

Edited by Stan Hu

Merge request reports