From 06e070811faa6ddd24b5cd67cce277136ed9d8a9 Mon Sep 17 00:00:00 2001 From: Justin Maier Date: Thu, 16 Jul 2026 19:47:57 -0600 Subject: [PATCH] fix(dump): sortAt must never trust the Image.sortAt column (no-backfill) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No-backfill decision: 92M historical Image rows stay un-backfilled, and their Image.sortAt column is NOT NULL but stale `now()`-at-migration garbage (measured on prod: 88.1% mismatch vs. the correct GREATEST value, clustering approx createdAt). The merged W2-1 dump copy_query used COALESCE(i."sortAt", GREATEST(...)) — COALESCE prefers the non-null column, so it would have ingested that garbage for ~92M rows. Fix: the dump computes sortAt as a PURE GREATEST(post.publishedAt, scannedAt, createdAt) and never reads i."sortAt" at all. Picked over a dump computed_field(max(...)) alternative because it's mechanically simpler and avoids a real limitation: dump top-level computed_fields evaluate via eval_indexed(indexed_fields_buf, ...) where indexed_fields_buf is filled from the raw CSV row (dump_processor.rs:1970) BEFORE enrichment runs (dump_processor.rs:1984) — so a computed_field cannot see the posts enrichment's publishedAtSecs without extending that code path. Pure SQL GREATEST needs no code changes. The steady-state ops emission expression (shared_ops_fields: [sortAtUnix], COALESCE(NEW."sortAt", GREATEST(...))) is UNCHANGED — it stays COALESCE-shaped because it's safe for a DIFFERENT reason: the model-share `image_sort_at_before` BEFORE INSERT OR UPDATE trigger recomputes NEW."sortAt" on every write that reaches PG, so any row this trigger fires on is correct-on-write by construction. The dump has no such guarantee (it reads all rows regardless of post-cutover write history). Pinned this pairing in comments in both trigger blocks: "ops emission may trust the column ONLY because image_sort_at_before recomputes it on every write; the dump must NEVER trust it (92M pre-trigger rows are stale)." Test (src/pg_sync/sync_config.rs, on the REAL deploy config): test_dump_sortat_never_trusts_the_column — dump copy_query has no COALESCE and no i."sortAt" read reference, computes sortAt via a type-correct extract(epoch from GREATEST(...))::bigint; ops trigger's sortAtUnix track field remains COALESCE-shaped (asserts the pairing didn't get flattened). cargo test --lib --features pg-sync: 1250 passed, 8 ignored (test_min_tracked_value_after_expansion skipped — known deadlock). Co-Authored-By: Claude Fable 5 --- deploy/configs/sync-config-civitai.yaml | 58 +++++++++++++----- src/pg_sync/sync_config.rs | 81 +++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 14 deletions(-) diff --git a/deploy/configs/sync-config-civitai.yaml b/deploy/configs/sync-config-civitai.yaml index 1056658..26269ee 100644 --- a/deploy/configs/sync-config-civitai.yaml +++ b/deploy/configs/sync-config-civitai.yaml @@ -38,22 +38,41 @@ dump_phases: table: Image # [PR-B3] sortAt: emitted directly as epoch SECONDS (bigint) so it lands in # the sortAt sort layers with no ms conversion (data_schema fallback: sortAt, - # which does NOT apply ms_to_seconds). TYPE-CORRECT belt — every COALESCE/ - # GREATEST operand is a TIMESTAMP, and extract(epoch ...)::bigint wraps the - # whole thing (never COALESCE(timestamp, bigint)). i."sortAt" is the value the - # model-share trigger maintains as GREATEST(post.publishedAt, scannedAt, - # createdAt); the GREATEST here is only the fallback for rows the backfill - # hasn't populated yet (COALESCE short-circuits, so the Post subselect runs - # only when i."sortAt" IS NULL). Matches the steady-state trigger's sortAtUnix - # value ÷ 1000 (W3 asserts dump == trigger on the same rows). + # which does NOT apply ms_to_seconds). TYPE-CORRECT: every GREATEST operand + # is a TIMESTAMP, and extract(epoch ...)::bigint wraps the whole expression. + # + # NO-BACKFILL DECISION (2026-07-16): the dump computes sortAt as a PURE + # GREATEST(post.publishedAt, scannedAt, createdAt) — it NEVER reads + # i."sortAt". A COALESCE(i."sortAt", GREATEST(...)) shape was tried first and + # is WRONG here: with no backfill, ~92M historical Image rows have a + # NOT-NULL i."sortAt" that is stale `now()`-at-migration garbage (measured on + # prod: 88.1% mismatch vs. the correct GREATEST value, clustering ≈ + # createdAt). COALESCE prefers the column when non-null, so it would have + # ingested that garbage for ~92M rows. Only rows the model-share + # `image_sort_at_before` trigger has WRITTEN THROUGH (post-cutover + # inserts/updates) have a trustworthy column value — the dump has no way to + # distinguish "trigger-written" from "stale migration default" from SQL + # alone, so it must ignore the column entirely and always recompute. + # + # PINNED PAIRING (do not decouple without re-reading this comment): the + # STEADY-STATE ops emission expression below (shared_ops_fields: + # [sortAtUnix], `COALESCE(NEW."sortAt", GREATEST(...))`) is UNCHANGED and + # stays COALESCE-shaped — trusting the column there is safe ONLY because + # `image_sort_at_before` (model-share BEFORE INSERT OR UPDATE trigger) + # recomputes NEW."sortAt" on every write that reaches PG, so any row the ops + # trigger fires on has a correct-on-write column by construction. The dump + # has no such guarantee (it reads rows regardless of whether they were ever + # touched post-cutover) — ops emission may trust the column ONLY because + # image_sort_at_before recomputes it on every write; the dump must NEVER + # trust it (92M pre-trigger rows are stale). copy_query: > COPY (SELECT i.id, i.url, i."nsfwLevel", i.hash, i.flags, i.type::text, i."userId", i."blockedFor", extract(epoch from i."scannedAt")::bigint as "scannedAtSecs", extract(epoch from i."createdAt")::bigint as "createdAtSecs", - extract(epoch from COALESCE(i."sortAt", + extract(epoch from GREATEST((SELECT p."publishedAt" FROM "Post" p WHERE p.id = i."postId"), - i."scannedAt", i."createdAt")))::bigint as "sortAt", + i."scannedAt", i."createdAt"))::bigint as "sortAt", i."postId", i.width, i.height FROM "Image" i) TO STDOUT WITH (FORMAT csv) @@ -239,10 +258,21 @@ triggers: # # sortAtUnix is emitted via the SHARED bitdex_image_sortat_ops() function so # the W1-3 re-emitter re-asserts the identical value (shape parity). MS units - # (`* 1000`) with a COALESCE(GREATEST...) belt for rows the PG backfill hasn't - # populated Image.sortAt on yet. Expression is byte-identical to the W1-1 - # codegen pin (trigger_gen.rs SORTATUNIX_EXPR); COALESCE short-circuits so the - # Post subselect only evaluates when Image.sortAt is still NULL. + # (`* 1000`) with a COALESCE(NEW."sortAt", GREATEST(...)) shape — trusting + # NEW."sortAt" here is UNCHANGED from before the no-backfill decision and + # stays COALESCE-shaped, because it is SAFE for a different reason than the + # dump: the model-share `image_sort_at_before` BEFORE INSERT OR UPDATE + # trigger recomputes Image.sortAt = GREATEST(post.publishedAt, scannedAt, + # createdAt) on EVERY write that reaches PG, so by the time this trigger + # fires, NEW."sortAt" is already correct-on-write (COALESCE's GREATEST + # fallback only exists for the pre-cutover window / defense-in-depth, not for + # 92M-stale-row coverage the way the old dump reasoning assumed). + # + # PINNED PAIRING — do not decouple without re-reading the dump's copy_query + # comment above: ops emission may trust the column ONLY because + # image_sort_at_before recomputes it on every write; the dump must NEVER + # trust it (92M pre-trigger rows are stale). Expression is byte-identical to + # the W1-1 codegen pin (trigger_gen.rs SORTATUNIX_EXPR). shared_ops_fields: [sortAtUnix] track_fields: - nsfwLevel diff --git a/src/pg_sync/sync_config.rs b/src/pg_sync/sync_config.rs index 35bfcff..a7b786c 100644 --- a/src/pg_sync/sync_config.rs +++ b/src/pg_sync/sync_config.rs @@ -966,4 +966,85 @@ triggers: [] assert!(ds_fields.iter().any(|f| f["target"].as_str() == Some("model3dId")), "model3dId must be in data_schema"); } + + /// No-backfill decision (2026-07-16): with ~92M historical Image rows never + /// backfilled, i."sortAt" is NOT-NULL-but-stale (measured on prod: 88.1% + /// mismatch vs. the correct GREATEST value, clustering ≈ createdAt — a + /// `now()`-at-migration default, not a computed value). A + /// `COALESCE(i."sortAt", GREATEST(...))` dump copy_query would prefer that + /// stale column and ingest garbage for ~92M rows. The dump must compute + /// sortAt as a PURE GREATEST and never read i."sortAt" at all. + /// + /// This is the mirror image of `test_image_sortatunix_emitted_ms_on_deploy_config`: + /// that test pins the OPS trigger's `COALESCE(NEW."sortAt", GREATEST(...))` + /// as correct (trusting the column is safe there because model-share's + /// `image_sort_at_before` BEFORE trigger recomputes it on every write this + /// trigger fires for). This test pins the DUMP side of the same pairing: + /// the two code paths trust the column differently ON PURPOSE, and a change + /// to one without re-reading the other's rationale is the failure mode this + /// guards against. + #[test] + fn test_dump_sortat_never_trusts_the_column() { + let config = load_deploy_sync_config(); + let images = config.dump_phases.iter().find(|p| p.name == "images") + .expect("images dump phase must exist"); + let copy_query = images.copy_query.as_deref() + .expect("images phase must have a copy_query"); + + // The dump must NEVER read i."sortAt" — only ever WRITE the alias + // `as "sortAt"` for the computed expression's output column. A read + // reference (`i."sortAt"` used as an operand, e.g. inside a COALESCE) + // is exactly the pattern this test guards against. + assert!( + !copy_query.contains(r#"i."sortAt""#), + "dump copy_query must not READ the sortAt column (92M pre-trigger \ + rows are stale) — found a read reference:\n{copy_query}" + ); + // No COALESCE anywhere near the sortAt computation — a COALESCE shape + // is exactly the wrong-for-this-world pattern this test guards against. + assert!( + !copy_query.to_uppercase().contains("COALESCE"), + "dump copy_query must not COALESCE onto the stale sortAt column:\n{copy_query}" + ); + // Must still compute sortAt via a pure GREATEST over publishedAt/ + // scannedAt/createdAt, aliased "sortAt", with type-correct casts + // (extract(epoch from ...)::bigint wraps the whole GREATEST — never + // GREATEST applied to a mix of timestamp and bigint operands). + assert!( + copy_query.contains("GREATEST(") && copy_query.contains(r#"as "sortAt""#), + "dump copy_query must compute sortAt via GREATEST(...) as \"sortAt\":\n{copy_query}" + ); + let normalized: String = copy_query.split_whitespace().collect::>().join(" "); + assert!( + normalized.contains("extract(epoch from GREATEST("), + "sortAt's extract(epoch from ...) must wrap the GREATEST(...) as a \ + single timestamp expression, not mix bigint/timestamp operands \ + [PR-B3]:\n{copy_query}" + ); + + // The `sortAt` CSV column is still declared and mapped to the sortAt + // sort field (unaffected by the no-backfill decision). + assert!(images.columns.iter().any(|c| c == "sortAt"), + "images phase columns must include sortAt"); + assert!(images.fields.iter().any(|f| f.target() == "sortAt"), + "images phase fields must map sortAt"); + + // PINNED PAIRING: the ops trigger's sortAtUnix expression is UNCHANGED + // and stays COALESCE-shaped — trusting the column there is a *different* + // safety argument (image_sort_at_before recomputes on every write), not + // a relaxation of the dump's no-trust rule. + let image_trigger = config.triggers.iter().find(|t| t.table == "Image") + .expect("Image trigger must exist"); + let sortatunix_track = image_trigger.track_fields.as_deref().unwrap_or(&[]) + .iter() + .map(|tf| tf.to_track_string()) + .find(|s| s.contains("sortAtUnix")) + .expect("Image trigger must have a sortAtUnix track field"); + assert!( + sortatunix_track.to_uppercase().contains("COALESCE"), + "ops emission sortAtUnix expression must remain COALESCE-shaped \ + (safe via image_sort_at_before recompute-on-write, unlike the \ + dump):\n{sortatunix_track}" + ); + } }