Skip to content

Commit e3ab972

Browse files
committed
fix: code review
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
1 parent 8184f3c commit e3ab972

2 files changed

Lines changed: 19 additions & 13 deletions

File tree

backend/src/osspckgs/migrations/V1784718693__merge_case_duplicate_github_repos.sql

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,15 @@ WHERE c.repo_id = m.repo_id
7070

7171
-- packages.repository_url is denormalized and must keep matching the
7272
-- canonical repos.url (the maven backfill updates the two atomically for the
73-
-- same reason). Match against member urls while the loser rows still exist;
74-
-- last_synced_at is the packages watermark, bumping it ships the correction
75-
-- to Tinybird.
73+
-- same reason). Joined on repos directly, not the merge map, so it also
74+
-- covers packages pointing at mixed-case singletons normalized below; runs
75+
-- while loser urls still exist to match against. last_synced_at is the
76+
-- packages watermark, bumping it ships the correction to Tinybird.
7677
UPDATE packages p
7778
SET repository_url = LOWER(p.repository_url), last_synced_at = NOW()
78-
FROM repo_merge_members m
79-
JOIN repos r ON r.id = m.repo_id
80-
WHERE p.repository_url = r.url
79+
FROM repos r
80+
WHERE r.host = 'github'
81+
AND p.repository_url = r.url
8182
AND p.repository_url <> LOWER(p.repository_url);
8283

8384
-- Cascades security_contacts and repo_activity_snapshot on losers.
@@ -86,14 +87,17 @@ USING repo_merge_members m
8687
WHERE r.id = m.repo_id
8788
AND NOT m.is_keeper;
8889

89-
-- Groups that had no lowercase variant: lowercase the surviving row. Safe
90-
-- against UNIQUE (url) — every row sharing LOWER(url) was in the same group,
91-
-- and its losers are gone by now.
90+
-- Lowercase every surviving mixed-case github url: keepers whose group had
91+
-- no lowercase variant, plus mixed-case singletons that never had a
92+
-- duplicate. Singletons must be normalized before the guard index exists —
93+
-- writers upsert with ON CONFLICT (url), which does not arbitrate on the
94+
-- LOWER(url) index, so a later insert of the same repo's canonical lowercase
95+
-- url (all writers lowercase now) would raise unique_violation instead of
96+
-- upserting. Safe against UNIQUE (url): any two rows sharing LOWER(url) were
97+
-- a group above, and their losers are gone by now.
9298
UPDATE repos r
9399
SET url = LOWER(r.url), updated_at = NOW()
94-
FROM repo_merge_members m
95-
WHERE r.id = m.repo_id
96-
AND m.is_keeper
100+
WHERE r.host = 'github'
97101
AND r.url <> LOWER(r.url);
98102

99103
-- The recurrence-guard unique index lives in the next migration: it must be

backend/src/osspckgs/migrations/V1784718694__repos_github_lower_url_guard.sql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
--
1414
-- A failed concurrent build leaves an INVALID index behind; the DROP lets a
1515
-- re-run replace it, where IF NOT EXISTS would silently keep the broken one.
16-
DROP INDEX IF EXISTS repos_github_lower_url_uq;
16+
-- Also CONCURRENTLY: a plain drop takes an ACCESS EXCLUSIVE lock on repos,
17+
-- stalling the workers on the retry path this exists for.
18+
DROP INDEX CONCURRENTLY IF EXISTS repos_github_lower_url_uq;
1719

1820
CREATE UNIQUE INDEX CONCURRENTLY repos_github_lower_url_uq
1921
ON repos (LOWER(url))

0 commit comments

Comments
 (0)