Fix GROUP BY in VSA label queries (!34381) · Merge requests · GitLab.org / GitLab

Adam Hegyi requested to merge fix-group-error-in-vsa-label-queries into master Jun 11, 2020

What does this MR do?

Bug:

VSA uses custom ordering when rendering the related issues and MRs (right side view). The order is configured to the end event timestamp (direction desc) of the stage.

Example:

IssuesFinder.new(current_user, { group_id: 12, # more params })
  .execute
  .order(stage.end_event => :desc)

This works well until we have label filters (more than 1 labels):

IssuesFinder.new(current_user, { group_id: 12, label_name: ['a', 'b'] })
  .execute
  .order(stage.end_event => :desc)

This will raise the error: ERROR: column "issue_metrics.first_mentioned_in_commit_at" must appear in the GROUP BY clause or be used in an aggregate function.

Reason:

Issue has many labels, when joining the label_links table and we make sure that we have two results from label_links. Example query:

select issues.id from issues inner join label_links on label_links.target_id=issues.id AND label_links.target_type = 'Issue' group by issues.id having count(label_links.id) = 2 limit 2

We get the error when we reference a field from other table:

select issues.id from issues inner join issue_metrics on issue_metrics.issue_id=issues.id inner join label_links on label_links.target_id=issues.id AND label_links.target_type = 'Issue' group by issues.id having count(label_links.id) = 2 order by issue_metrics.id limit 2

Fix: add the extra column(s) within the ORDER BY and SELECT in the GROUP BY clause

select issues.id from issues inner join issue_metrics on issue_metrics.issue_id=issues.id inner join label_links on label_links.target_id=issues.id AND label_links.target_type = 'Issue' group by issues.id, issue_metrics.id having count(label_links.id) = 2 order by issue_metrics.id limit 2

The change adds an extra method to all VSA stage event definition where we need to explicitly define which extra columns are used for the stage (#column_list).

Example query with the fix:

SELECT "issues"."title",
       "issues"."iid",
       "issues"."id",
       "issues"."created_at",
       "issues"."author_id",
       "issues"."project_id",
       ROUND(EXTRACT(EPOCH
                     FROM COALESCE("issue_metrics"."first_associated_with_milestone_at", "issue_metrics"."first_added_to_board_at") - "issues"."created_at")) AS total_time
FROM "issues"
INNER JOIN "projects" ON "projects"."id" = "issues"."project_id"
INNER JOIN "label_links" ON "label_links"."target_type" = 'Issue'
AND "label_links"."target_id" = "issues"."id"
INNER JOIN "labels" ON "labels"."id" = "label_links"."label_id"
INNER JOIN "issue_metrics" ON "issue_metrics"."issue_id" = "issues"."id"
LEFT JOIN project_features ON projects.id = project_features.project_id
WHERE "projects"."namespace_id" IN
    (WITH RECURSIVE "base_and_descendants" AS (
                                                 (SELECT "namespaces".*
                                                  FROM "namespaces"
                                                  WHERE "namespaces"."type" = 'Group'
                                                    AND "namespaces"."id" = 9970)
                                               UNION
                                                 (SELECT "namespaces".*
                                                  FROM "namespaces",
                                                       "base_and_descendants"
                                                  WHERE "namespaces"."type" = 'Group'
                                                    AND "namespaces"."parent_id" = "base_and_descendants"."id")) SELECT "namespaces"."id"
     FROM "base_and_descendants" AS "namespaces")
  AND (EXISTS
         (SELECT 1
          FROM "project_authorizations"
          WHERE "project_authorizations"."user_id" = 4156052
            AND (project_authorizations.project_id = projects.id))
       OR projects.visibility_level IN (0,
                                        10,
                                        20))
  AND ("project_features"."issues_access_level" > 0
       OR "project_features"."issues_access_level" IS NULL)
  AND "issues"."created_at" <= '2020-06-15 23:59:59.999999'
  AND "issues"."created_at" >= '2020-05-17 00:00:00'
  AND "labels"."title" IN ('feature',
                           'devops::manage')
  AND ("issue_metrics"."first_added_to_board_at" IS NOT NULL
       OR "issue_metrics"."first_associated_with_milestone_at" IS NOT NULL)
  AND COALESCE("issue_metrics"."first_associated_with_milestone_at", "issue_metrics"."first_added_to_board_at") >= "issues"."created_at"
GROUP BY "issues"."id",
         "issue_metrics"."first_associated_with_milestone_at",
         "issue_metrics"."first_added_to_board_at"
HAVING (COUNT(DISTINCT labels.title) = 2)
ORDER BY COALESCE("issue_metrics"."first_associated_with_milestone_at", "issue_metrics"."first_added_to_board_at") DESC
LIMIT 20

I added ~backstage label since the filter is not user facing (FE implementation is still WIP).

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
Tested in all supported browsers
Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

Label as security and @ mention @gitlab-com/gl-security/appsec
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
Security reports checked/validated by a reviewer from the AppSec team

Edited Oct 29, 2020 by 🤖 GitLab Bot 🤖

Fix GROUP BY in VSA label queries