Extend MergeRequestFinder to search by squash and merge commits
What does this MR do?
Contributes to #21686 (closed)
Problem
Link to the related MR is missing for squashed and merge commits.
Solution
Extend MergeRequestFinder to match not only commits that belong to the merge request but also related squash and merge commits.
Main changes
- Add database indices for
squash_commit_shaandmerge_commit_shafields - Add new scope
by_related_commit_shato MergeRequest model - Update MergeRequestFinder logic to use
by_related_commit_shascope for filtering
Database
Migrations
Up
== 20201216151616 AddSquashCommitShaIndex: migrating ==========================
-- transaction_open?()
-> 0.0000s
-- index_exists?(:merge_requests, [:target_project_id, :squash_commit_sha], {:name=>"index_merge_requests_on_target_project_id_and_squash_commit_sha", :algorithm=>:concurrently})
-> 0.0086s
-- execute("SET statement_timeout TO 0")
-> 0.0001s
-- add_index(:merge_requests, [:target_project_id, :squash_commit_sha], {:name=>"index_merge_requests_on_target_project_id_and_squash_commit_sha", :algorithm=>:concurrently})
-> 0.0056s
-- execute("RESET ALL")
-> 0.0001s
== 20201216151616 AddSquashCommitShaIndex: migrated (0.0149s) =================== 20201216155333 AddMergeCommitShaIndex: migrating ===========================
-- transaction_open?()
-> 0.0000s
-- index_exists?(:merge_requests, [:target_project_id, :merge_commit_sha], {:name=>"index_merge_requests_on_target_project_id_and_merge_commit_sha", :algorithm=>:concurrently})
-> 0.0067s
-- add_index(:merge_requests, [:target_project_id, :merge_commit_sha], {:name=>"index_merge_requests_on_target_project_id_and_merge_commit_sha", :algorithm=>:concurrently})
-> 0.0022s
== 20201216155333 AddMergeCommitShaIndex: migrated (0.0092s) ==================Down
== 20201216155333 AddMergeCommitShaIndex: reverting ===========================
-- transaction_open?()
-> 0.0000s
-- index_exists?(:merge_requests, [:target_project_id, :merge_commit_sha], {:name=>"index_merge_requests_on_target_project_id_and_merge_commit_sha", :algorithm=>:concurrently})
-> 0.0109s
-- execute("SET statement_timeout TO 0")
-> 0.0001s
-- remove_index(:merge_requests, {:name=>"index_merge_requests_on_target_project_id_and_merge_commit_sha", :algorithm=>:concurrently, :column=>[:target_project_id, :merge_commit_sha]})
-> 0.0123s
-- execute("RESET ALL")
-> 0.0002s
== 20201216155333 AddMergeCommitShaIndex: reverted (0.0240s) ==================== 20201216151616 AddSquashCommitShaIndex: reverting ==========================
-- transaction_open?()
-> 0.0000s
-- index_exists?(:merge_requests, [:target_project_id, :squash_commit_sha], {:name=>"index_merge_requests_on_target_project_id_and_squash_commit_sha", :algorithm=>:concurrently})
-> 0.0108s
-- execute("SET statement_timeout TO 0")
-> 0.0001s
-- remove_index(:merge_requests, {:name=>"index_merge_requests_on_target_project_id_and_squash_commit_sha", :algorithm=>:concurrently, :column=>[:target_project_id, :squash_commit_sha]})
-> 0.0090s
-- execute("RESET ALL")
-> 0.0001s
== 20201216151616 AddSquashCommitShaIndex: reverted (0.0205s) =================Query changes
MergeRequestFinder
MergeRequestsFinder.new(nil, { commit_sha: '6c3984244f2b4d39396d70551f8256955bd876f9' }).executeBefore
MergeRequest Load (1.7ms) SELECT "merge_requests".* FROM "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 "projects"."visibility_level" = 20 AND ("project_features"."merge_requests_access_level" IN (20, 30) OR "project_features"."merge_requests_access_level" IS NULL) AND (EXISTS (SELECT 1 FROM "merge_request_diffs" INNER JOIN "merge_request_diff_commits" ON "merge_request_diff_commits"."merge_request_diff_id" = "merge_request_diffs"."id" WHERE (merge_requests.latest_merge_request_diff_id = merge_request_diffs.id) AND "merge_request_diff_commits"."sha" = '\x6c3984244f2b4d39396d70551f8256955bd876f9')) ORDER BY "merge_requests"."id" DESCAfter
MergeRequest Load (4.7ms) SELECT "merge_requests".* FROM ((SELECT "merge_requests".* FROM "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 "projects"."visibility_level" = 20 AND ("project_features"."merge_requests_access_level" IN (20, 30) OR "project_features"."merge_requests_access_level" IS NULL) AND (EXISTS (SELECT 1 FROM "merge_request_diffs" INNER JOIN "merge_request_diff_commits" ON "merge_request_diff_commits"."merge_request_diff_id" = "merge_request_diffs"."id" WHERE (merge_requests.latest_merge_request_diff_id = merge_request_diffs.id) AND "merge_request_diff_commits"."sha" = '\x6c3984244f2b4d39396d70551f8256955bd876f9')))
UNION
(SELECT "merge_requests".* FROM "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 "projects"."visibility_level" = 20 AND ("project_features"."merge_requests_access_level" IN (20, 30) OR "project_features"."merge_requests_access_level" IS NULL) AND "merge_requests"."squash_commit_sha" = '\x6c3984244f2b4d39396d70551f8256955bd876f9')
UNION
(SELECT "merge_requests".* FROM "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 "projects"."visibility_level" = 20 AND ("project_features"."merge_requests_access_level" IN (20, 30) OR "project_features"."merge_requests_access_level" IS NULL) AND "merge_requests"."merge_commit_sha" = '6c3984244f2b4d39396d70551f8256955bd876f9')) 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 "projects"."visibility_level" = 20 AND ("project_features"."merge_requests_access_level" IN (20, 30) OR "project_features"."merge_requests_access_level" IS NULL) ORDER BY "merge_requests"."id" DESCExplanation Search query was extended to match commit hashes from "merge_requests"."merge_commit_sha" and "merge_requests"."squash_commit_sha" fields.
Query Plans
Before
https://explain.depesz.com/s/UdKD
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort (cost=64.54..64.59 rows=19 width=696) (actual time=0.066..0.069 rows=0 loops=1)
Sort Key: merge_requests.id DESC
Sort Method: quicksort Memory: 25kB
-> Hash Left Join (cost=48.45..64.14 rows=19 width=696) (actual time=0.062..0.064 rows=0 loops=1)
Hash Cond: (projects.id = project_features.project_id)
Filter: ((project_features.merge_requests_access_level = ANY ('{20,30}'::integer[])) OR (project_features.merge_requests_access_level IS NULL))
-> Hash Join (cost=47.02..62.64 rows=20 width=700) (actual time=0.062..0.063 rows=0 loops=1)
Hash Cond: (merge_requests.target_project_id = projects.id)
-> Merge Semi Join (cost=44.59..60.08 rows=41 width=696) (actual time=0.035..0.036 rows=0 loops=1)
Merge Cond: (merge_requests.latest_merge_request_diff_id = merge_request_diff_commits.merge_request_diff_id)
-> Index Scan using index_merge_requests_on_latest_merge_request_diff_id on merge_requests (cost=0.14..8.08 rows=63 width=696) (actual time=0.007..0.007 rows=1 loops=1)
-> Merge Join (cost=44.45..53.76 rows=41 width=8) (actual time=0.027..0.027 rows=0 loops=1)
Merge Cond: (merge_request_diffs.id = merge_request_diff_commits.merge_request_diff_id)
-> Index Only Scan using merge_request_diffs_pkey on merge_request_diffs (cost=0.14..10.57 rows=162 width=4) (actual time=0.013..0.013 rows=1 loops=1)
Heap Fetches: 1
-> Sort (cost=44.24..44.34 rows=41 width=4) (actual time=0.013..0.013 rows=0 loops=1)
Sort Key: merge_request_diff_commits.merge_request_diff_id
Sort Method: quicksort Memory: 25kB
-> Index Scan using index_merge_request_diff_commits_on_sha on merge_request_diff_commits (cost=0.42..43.14 rows=41 width=4) (actual time=0.009..0.009 rows=0 loops=1)
Index Cond: (sha = '\x6c3984244f2b4d39396d70551f8256955bd876f9'::bytea)
-> Hash (cost=2.30..2.30 rows=11 width=4) (actual time=0.017..0.018 rows=10 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 9kB
-> Index Only Scan using index_projects_api_vis20_updated_at on projects (cost=0.14..2.30 rows=11 width=4) (actual time=0.009..0.013 rows=10 loops=1)
Heap Fetches: 9
-> Hash (cost=1.19..1.19 rows=19 width=8) (never executed)
-> Seq Scan on project_features (cost=0.00..1.19 rows=19 width=8) (never executed)
Planning Time: 1.247 ms
Execution Time: 0.165 ms
(28 rows)After
https://explain.depesz.com/s/IqYg
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort (cost=84.18..84.20 rows=9 width=614) (actual time=0.124..0.131 rows=1 loops=1)
Sort Key: merge_requests.id DESC
Sort Method: quicksort Memory: 25kB
-> Hash Left Join (cost=81.70..84.04 rows=9 width=614) (actual time=0.116..0.123 rows=1 loops=1)
Hash Cond: (projects.id = project_features.project_id)
Filter: ((project_features.merge_requests_access_level = ANY ('{20,30}'::integer[])) OR (project_features.merge_requests_access_level IS NULL))
-> Hash Join (cost=80.28..82.58 rows=10 width=618) (actual time=0.093..0.099 rows=1 loops=1)
Hash Cond: (projects.id = merge_requests.target_project_id)
-> Index Only Scan using index_projects_api_vis20_updated_at on projects (cost=0.14..2.30 rows=11 width=4) (actual time=0.005..0.008 rows=10 loops=1)
Heap Fetches: 9
-> Hash (cost=79.88..79.88 rows=21 width=614) (actual time=0.079..0.084 rows=1 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 9kB
-> HashAggregate (cost=79.46..79.67 rows=21 width=614) (actual time=0.077..0.082 rows=1 loops=1)
Group Key: merge_requests.id, merge_requests.target_branch, merge_requests.source_branch, merge_requests.source_project_id, merge_requests.author_id, merge_requests.assignee_id, merge_requests.title, merge_requests.created_at, merge_requests.updated_at, merge_requests.milestone_id, merge_requests.merge_status, merge_requests.target_project_id, merge_requests.iid, merge_requests.description, merge_requests.updated_by_id, merge_requests.merge_error, merge_requests.merge_params, merge_requests.merge_when_pipeline_succeeds, merge_requests.merge_user_id, merge_requests.merge_commit_sha, merge_requests.approvals_before_merge, merge_requests.rebase_commit_sha, merge_requests.in_progress_merge_commit_sha, merge_requests.lock_version, merge_requests.title_html, merge_requests.description_html, merge_requests.time_estimate, merge_requests.squash, merge_requests.cached_markdown_version, merge_requests.last_edited_at, merge_requests.last_edited_by_id, merge_requests.head_pipeline_id, merge_requests.merge_jid, merge_requests.discussion_locked, merge_requests.latest_merge_request_diff_id, merge_requests.allow_maintainer_to_push, merge_requests.state_id, merge_requests.rebase_jid, merge_requests.squash_commit_sha, merge_requests.sprint_id, merge_requests.merge_ref_sha
-> Append (cost=48.45..77.31 rows=21 width=614) (actual time=0.067..0.073 rows=1 loops=1)
-> Hash Left Join (cost=48.45..64.14 rows=19 width=696) (actual time=0.031..0.033 rows=0 loops=1)
Hash Cond: (projects_1.id = project_features_1.project_id)
Filter: ((project_features_1.merge_requests_access_level = ANY ('{20,30}'::integer[])) OR (project_features_1.merge_requests_access_level IS NULL))
-> Hash Join (cost=47.02..62.64 rows=20 width=700) (actual time=0.031..0.033 rows=0 loops=1)
Hash Cond: (merge_requests.target_project_id = projects_1.id)
-> Merge Semi Join (cost=44.59..60.08 rows=41 width=696) (actual time=0.017..0.018 rows=0 loops=1)
Merge Cond: (merge_requests.latest_merge_request_diff_id = merge_request_diff_commits.merge_request_diff_id)
-> Index Scan using index_merge_requests_on_latest_merge_request_diff_id on merge_requests (cost=0.14..8.08 rows=63 width=696) (actual time=0.003..0.003 rows=1 loops=1)
-> Merge Join (cost=44.45..53.76 rows=41 width=8) (actual time=0.012..0.013 rows=0 loops=1)
Merge Cond: (merge_request_diffs.id = merge_request_diff_commits.merge_request_diff_id)
-> Index Only Scan using merge_request_diffs_pkey on merge_request_diffs (cost=0.14..10.57 rows=162 width=4) (actual time=0.004..0.004 rows=1 loops=1)
Heap Fetches: 1
-> Sort (cost=44.24..44.34 rows=41 width=4) (actual time=0.007..0.007 rows=0 loops=1)
Sort Key: merge_request_diff_commits.merge_request_diff_id
Sort Method: quicksort Memory: 25kB
-> Index Scan using index_merge_request_diff_commits_on_sha on merge_request_diff_commits (cost=0.42..43.14 rows=41 width=4) (actual time=0.005..0.005 rows=0 loops=1)
Index Cond: (sha = '\x6c3984244f2b4d39396d70551f8256955bd876f9'::bytea)
-> Hash (cost=2.30..2.30 rows=11 width=4) (actual time=0.010..0.010 rows=10 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 9kB
-> Index Only Scan using index_projects_api_vis20_updated_at on projects projects_1 (cost=0.14..2.30 rows=11 width=4) (actual time=0.002..0.007 rows=10 loops=1)
Heap Fetches: 9
-> Hash (cost=1.19..1.19 rows=19 width=8) (never executed)
-> Seq Scan on project_features project_features_1 (cost=0.00..1.19 rows=19 width=8) (never executed)
-> Nested Loop Left Join (cost=3.91..6.43 rows=1 width=696) (actual time=0.006..0.007 rows=0 loops=1)
Filter: ((project_features_2.merge_requests_access_level = ANY ('{20,30}'::integer[])) OR (project_features_2.merge_requests_access_level IS NULL))
-> Hash Join (cost=3.77..5.99 rows=1 width=700) (actual time=0.006..0.007 rows=0 loops=1)
Hash Cond: (projects_2.id = merge_requests_1.target_project_id)
-> Index Only Scan using index_projects_api_vis20_updated_at on projects projects_2 (cost=0.14..2.30 rows=11 width=4) (actual time=0.002..0.002 rows=1 loops=1)
Heap Fetches: 1
-> Hash (cost=3.62..3.62 rows=1 width=696) (actual time=0.003..0.003 rows=0 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 8kB
-> Index Scan using index_merge_requests_on_target_project_id_and_squash_commit_sha on merge_requests merge_requests_1 (cost=0.14..3.62 rows=1 width=696) (actual time=0.002..0.003 rows=0 loops=1)
Index Cond: (squash_commit_sha = '\x6c3984244f2b4d39396d70551f8256955bd876f9'::bytea)
-> Index Scan using index_project_features_on_project_id on project_features project_features_2 (cost=0.14..0.43 rows=1 width=8) (never executed)
Index Cond: (projects_2.id = project_id)
-> Nested Loop Left Join (cost=3.91..6.43 rows=1 width=696) (actual time=0.028..0.030 rows=1 loops=1)
Filter: ((project_features_3.merge_requests_access_level = ANY ('{20,30}'::integer[])) OR (project_features_3.merge_requests_access_level IS NULL))
-> Hash Join (cost=3.77..5.99 rows=1 width=700) (actual time=0.018..0.020 rows=1 loops=1)
Hash Cond: (projects_3.id = merge_requests_2.target_project_id)
-> Index Only Scan using index_projects_api_vis20_updated_at on projects projects_3 (cost=0.14..2.30 rows=11 width=4) (actual time=0.001..0.005 rows=10 loops=1)
Heap Fetches: 9
-> Hash (cost=3.62..3.62 rows=1 width=696) (actual time=0.005..0.005 rows=1 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 9kB
-> Index Scan using index_merge_requests_on_target_project_id_and_merge_commit_sha on merge_requests merge_requests_2 (cost=0.14..3.62 rows=1 width=696) (actual time=0.003..0.004 rows=1 loops=1)
Index Cond: ((merge_commit_sha)::text = '6c3984244f2b4d39396d70551f8256955bd876f9'::text)
-> Index Scan using index_project_features_on_project_id on project_features project_features_3 (cost=0.14..0.43 rows=1 width=8) (actual time=0.006..0.006 rows=1 loops=1)
Index Cond: (projects_3.id = project_id)
-> Hash (cost=1.19..1.19 rows=19 width=8) (actual time=0.015..0.015 rows=23 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 9kB
-> Seq Scan on project_features (cost=0.00..1.19 rows=19 width=8) (actual time=0.006..0.010 rows=23 loops=1)
Planning Time: 3.242 ms
Execution Time: 0.332 ms
(67 rows)Screenshots (strongly suggested)
| Squashed commit | Merge commit |
|---|---|
 |
 |
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
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 by Vasilii Iakliushin