Skip to content

Fix invalid cross-join in Security::Finding.by_build_ids

Dylan Griffith requested to merge 342366-fix-cross-join-security-findings-by-build-ids into master

What does this MR do and why?

Security::Finding.by_build_ids was doing a cross-join through ci_builds. This will not be allowed when ci_* tables moves to a new database. This is only used in JobArtifacts::DestroyBatchService and this is (luckily) already not using a subquery.

This is a redundant join anyway since security_scans table already has the build_id so we can simplify the query to reduce the extra join to ci_builds.

If the by_build_ids is ever used in future with a relation that has a subjoin then this will create a new cross join but today it's only used in one place with an array of ids so we don't need the cross-join.

Old query:

SELECT "security_findings".* FROM "security_findings"
INNER JOIN "security_scans" ON "security_scans"."id" =
"security_findings"."scan_id"
INNER JOIN "ci_builds" ON "ci_builds"."id" = "security_scans"."build_id"
AND "ci_builds"."type" = 'Ci::Build'
WHERE "ci_builds"."id" IN (1,2,3)

New query:

SELECT "security_findings".* FROM "security_findings"
INNER JOIN "security_scans" ON "security_scans"."id" =
"security_findings"."scan_id"
WHERE "security_findsings"."build_id" IN (1,2,3)

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Closes #342366 (closed)

Edited by Dylan Griffith

Merge request reports