Skip to content

Last push widget will show banner for new pushes to previously merged branch

What does this MR do?

Fixes #40818 (closed). Previously, the last push widget wouldn't display if there were any previous merge requests for the same source and target branch, even if they were merged or closed. It's perfectly reasonable that an existing branch would have a previous merged or closed merge request and push new commits later. This allows that to happen.

By changing this query it may seem that we would potentially show the banner after a MR for a branch has been merged/closed within the 2 hour window that the last push widget displays. This is avoided since the widget relies on the cached value which is removed when the event is first displayed. A new push will be required to add the cache value again.

EXPLAIN ANALYZE of existing query

EXPLAIN ANALYZE SELECT  "events".* FROM "events" INNER JOIN "push_event_payloads" ON "push_event_payloads"."event_id" = "events"."id" WHERE "events"."action" IN (5) AND (NOT EXISTS (SELECT 1 FROM "merge_requests" WHERE "merge_requests"."deleted_at" IS NULL AND (merge_requests.source_project_id = events.project_id) AND (merge_requests.source_branch = push_event_payloads.ref))) AND "push_event_payloads"."action" IN (2, 0) AND "push_event_payloads"."ref_type" = 0 AND "events"."id" = 1080 LIMIT 1;
                                                                                  QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.57..8.66 rows=1 width=39) (actual time=0.031..0.031 rows=1 loops=1)
   ->  Nested Loop Anti Join  (cost=0.57..8.66 rows=1 width=39) (actual time=0.031..0.031 rows=1 loops=1)
         Join Filter: (merge_requests.source_project_id = events.project_id)
         ->  Nested Loop  (cost=0.43..4.48 rows=1 width=71) (actual time=0.023..0.023 rows=1 loops=1)
               ->  Index Scan using index_events_on_action on events  (cost=0.28..2.30 rows=1 width=39) (actual time=0.016..0.016 rows=1 loops=1)
                     Index Cond: (action = 5)
                     Filter: (id = 1080)
               ->  Index Scan using index_push_event_payloads_on_event_id on push_event_payloads  (cost=0.15..2.17 rows=1 width=36) (actual time=0.005..0.005 rows=1 loops=1)
                     Index Cond: (event_id = 1080)
                     Filter: ((action = ANY ('{2,0}'::integer[])) AND (ref_type = 0))
         ->  Index Scan using index_merge_requests_on_source_branch on merge_requests  (cost=0.14..2.16 rows=1 width=17) (actual time=0.003..0.003 rows=0 loops=1)
               Index Cond: ((source_branch)::text = push_event_payloads.ref)
               Filter: (deleted_at IS NULL)
 Planning time: 0.452 ms
 Execution time: 0.125 ms
(15 rows)

EXPLAIN ANAYLIZE of new query

EXPLAIN ANALYZE SELECT  "events".* FROM "events" INNER JOIN "push_event_payloads" ON "push_event_payloads"."event_id" = "events"."id" WHERE "events"."action" IN (5) AND (NOT EXISTS (SELECT 1 FROM "merge_requests" WHERE "merge_requests"."deleted_at" IS NULL AND (merge_requests.source_project_id = events.project_id) AND (merge_requests.source_branch = push_event_payloads.ref) AND "merge_requests"."state" = 'opened')) AND "push_event_payloads"."action" IN (2, 0) AND "push_event_payloads"."ref_type" = 0 AND "events"."id" = 1081 LIMIT 1;
                                                                                  QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.57..8.67 rows=1 width=39) (actual time=0.030..0.030 rows=1 loops=1)
   ->  Nested Loop Anti Join  (cost=0.57..8.67 rows=1 width=39) (actual time=0.029..0.029 rows=1 loops=1)
         Join Filter: (merge_requests.source_project_id = events.project_id)
         ->  Nested Loop  (cost=0.43..4.48 rows=1 width=71) (actual time=0.021..0.021 rows=1 loops=1)
               ->  Index Scan using index_events_on_action on events  (cost=0.28..2.30 rows=1 width=39) (actual time=0.015..0.015 rows=1 loops=1)
                     Index Cond: (action = 5)
                     Filter: (id = 1081)
               ->  Index Scan using index_push_event_payloads_on_event_id on push_event_payloads  (cost=0.15..2.17 rows=1 width=36) (actual time=0.005..0.005 rows=1 loops=1)
                     Index Cond: (event_id = 1081)
                     Filter: ((action = ANY ('{2,0}'::integer[])) AND (ref_type = 0))
         ->  Index Scan using index_merge_requests_on_source_branch on merge_requests  (cost=0.14..2.16 rows=1 width=17) (actual time=0.003..0.003 rows=0 loops=1)
               Index Cond: ((source_branch)::text = push_event_payloads.ref)
               Filter: ((deleted_at IS NULL) AND ((state)::text = 'opened'::text))
 Planning time: 0.315 ms
 Execution time: 0.116 ms
(15 rows)

Are there points in the code the reviewer needs to double check?

@yorickpeterse Do you see any performance implications by adding the .where.not()? You reworked this portion of code a few months ago for performance reasons so I want to check with you here.

Why was this MR needed?

Customer reported this as a regression. I'm not 100% sure if it is, but in any case it seems like a reasonable use-case. If this is all that's required to support it then we should.

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #40818 (closed)

Edited by Drew Blessing

Merge request reports