fix: make the genre dedup migration collision-proof (0038)
Review caught a boot-crash risk: 0038's repoint-UPDATE could set two non-canonical variant links of one manga to the same canonical id in a single statement (NOT EXISTS sees the pre-statement snapshot) -> manga_genres PK violation -> migration rollback -> startup failure. Replace with INSERT ... SELECT DISTINCT ... ON CONFLICT DO NOTHING (collision-proof) then delete variants. Amended in place (0038 is unpushed). Tested with the exact collision scenario. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.128.24"
|
||||
version = "0.128.25"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.128.24"
|
||||
version = "0.128.25"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -9,32 +9,31 @@
|
||||
|
||||
-- Heal any pre-existing case-variant duplicates before adding the index, so
|
||||
-- `sqlx::migrate!` can't crash on a dirty table (mirrors 0031's pre-dedup).
|
||||
-- Keep the lowest id per lower(name) as canonical and repoint manga_genres.
|
||||
UPDATE manga_genres mg
|
||||
SET genre_id = d.keep_id
|
||||
FROM (
|
||||
SELECT g.id AS dup_id, k.keep_id
|
||||
FROM genres g
|
||||
JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id
|
||||
FROM genres GROUP BY lower(name)) k
|
||||
ON lower(g.name) = k.lname AND g.id <> k.keep_id
|
||||
) d
|
||||
WHERE mg.genre_id = d.dup_id
|
||||
AND NOT EXISTS (
|
||||
SELECT 1 FROM manga_genres x
|
||||
WHERE x.manga_id = mg.manga_id AND x.genre_id = d.keep_id
|
||||
);
|
||||
-- Canonical = lowest id per lower(name).
|
||||
|
||||
-- Drop links that would have collided with the canonical link (already present).
|
||||
-- 1. Ensure every manga linked to any case-variant also has the canonical link.
|
||||
-- INSERT ... ON CONFLICT DO NOTHING is collision-proof: SELECT DISTINCT
|
||||
-- collapses multiple variants of one manga to a single canonical row, and the
|
||||
-- ON CONFLICT absorbs a canonical link that already exists. (A prior
|
||||
-- UPDATE-repoint could set two non-canonical rows of the SAME manga to the
|
||||
-- same canonical id in a single statement and violate the manga_genres PK,
|
||||
-- rolling the migration back and wedging startup.)
|
||||
INSERT INTO manga_genres (manga_id, genre_id)
|
||||
SELECT DISTINCT mg.manga_id, k.keep_id
|
||||
FROM manga_genres mg
|
||||
JOIN genres g ON mg.genre_id = g.id
|
||||
JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id
|
||||
FROM genres GROUP BY lower(name)) k
|
||||
ON lower(g.name) = k.lname
|
||||
WHERE g.id <> k.keep_id
|
||||
ON CONFLICT (manga_id, genre_id) DO NOTHING;
|
||||
|
||||
-- 2. Every non-canonical link now has a canonical sibling — drop the variants.
|
||||
DELETE FROM manga_genres mg
|
||||
USING (
|
||||
SELECT g.id AS dup_id
|
||||
FROM genres g
|
||||
JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id
|
||||
FROM genres GROUP BY lower(name)) k
|
||||
ON lower(g.name) = k.lname AND g.id <> k.keep_id
|
||||
) d
|
||||
WHERE mg.genre_id = d.dup_id;
|
||||
USING genres g,
|
||||
(SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id
|
||||
FROM genres GROUP BY lower(name)) k
|
||||
WHERE mg.genre_id = g.id AND lower(g.name) = k.lname AND g.id <> k.keep_id;
|
||||
|
||||
-- Remove the now-orphaned duplicate genre rows.
|
||||
DELETE FROM genres g
|
||||
|
||||
@@ -674,6 +674,93 @@ async fn arbitrary_genres_from_source_get_inserted(pool: PgPool) {
|
||||
assert_eq!(webtoons_count.0, 1, "case-insensitive lookup reuses the existing row");
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn genre_dedup_survives_a_manga_linked_to_two_variants(pool: PgPool) {
|
||||
// Regression for a migration-0038 collision: a manga linked to TWO
|
||||
// non-canonical case-variants of one genre. A repoint-UPDATE would set both
|
||||
// rows to the canonical id in one statement -> manga_genres PK violation ->
|
||||
// migration rollback -> boot failure. #[sqlx::test] migrates a clean DB, so
|
||||
// recreate the dirty pre-index state and replay 0038's healing statements;
|
||||
// they must complete and leave exactly the one canonical link. (Mirrors the
|
||||
// healing SQL in migrations/0038_genres_name_lower_unique.sql.)
|
||||
let manga = Uuid::new_v4();
|
||||
sqlx::query("INSERT INTO mangas (id, title) VALUES ($1, 'T')")
|
||||
.bind(manga)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Drop the live guard so we can plant case-variant genres.
|
||||
sqlx::query("DROP INDEX genres_name_lower_uniq").execute(&pool).await.unwrap();
|
||||
let keep = Uuid::parse_str("00000000-0000-0000-0000-000000000001").unwrap();
|
||||
let dup2 = Uuid::parse_str("00000000-0000-0000-0000-000000000002").unwrap();
|
||||
let dup3 = Uuid::parse_str("00000000-0000-0000-0000-000000000003").unwrap();
|
||||
for (id, name) in [(keep, "Zzz"), (dup2, "zzz"), (dup3, "ZZZ")] {
|
||||
sqlx::query("INSERT INTO genres (id, name) VALUES ($1, $2)")
|
||||
.bind(id)
|
||||
.bind(name)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
// The manga links to the two NON-canonical variants, not the canonical one.
|
||||
for gid in [dup2, dup3] {
|
||||
sqlx::query("INSERT INTO manga_genres (manga_id, genre_id) VALUES ($1, $2)")
|
||||
.bind(manga)
|
||||
.bind(gid)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
// Step 1 — collision-proof canonical-link backfill (the crux of the fix).
|
||||
sqlx::query(
|
||||
"INSERT INTO manga_genres (manga_id, genre_id) \
|
||||
SELECT DISTINCT mg.manga_id, k.keep_id \
|
||||
FROM manga_genres mg JOIN genres g ON mg.genre_id = g.id \
|
||||
JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id \
|
||||
FROM genres GROUP BY lower(name)) k ON lower(g.name) = k.lname \
|
||||
WHERE g.id <> k.keep_id \
|
||||
ON CONFLICT (manga_id, genre_id) DO NOTHING",
|
||||
)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("collision-proof INSERT must not raise a manga_genres PK violation");
|
||||
// Step 2 — drop the non-canonical links.
|
||||
sqlx::query(
|
||||
"DELETE FROM manga_genres mg USING genres g, \
|
||||
(SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id \
|
||||
FROM genres GROUP BY lower(name)) k \
|
||||
WHERE mg.genre_id = g.id AND lower(g.name) = k.lname AND g.id <> k.keep_id",
|
||||
)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
// Step 3 — remove orphaned duplicate genres.
|
||||
sqlx::query(
|
||||
"DELETE FROM genres g USING \
|
||||
(SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id \
|
||||
FROM genres GROUP BY lower(name)) k \
|
||||
WHERE lower(g.name) = k.lname AND g.id <> k.keep_id",
|
||||
)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Exactly the canonical link remains, and the guard re-creates cleanly.
|
||||
let links: Vec<Uuid> =
|
||||
sqlx::query_scalar("SELECT genre_id FROM manga_genres WHERE manga_id = $1")
|
||||
.bind(manga)
|
||||
.fetch_all(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(links, vec![keep], "manga keeps exactly the canonical genre link");
|
||||
sqlx::query("CREATE UNIQUE INDEX genres_name_lower_uniq ON genres (lower(name))")
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("no residual case-variant duplicates remain");
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn genres_reject_case_variant_duplicates_at_the_db(pool: PgPool) {
|
||||
// The sequential pre-check in sync_genres dedups the common case, but two
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.128.24",
|
||||
"version": "0.128.25",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
Reference in New Issue
Block a user