← back to Homesonspec
TK-10878 Phase 2 drafts: fix 3 fatal + 2 hardening review findings
ddb43e6722cffddda8a262a430e391f6b2f4e0bb · 2026-08-31 12:14:11 -0700 · Steve
- p_type: 'native' is invalid; default to 'range' (v5) with flagged v4
'partman' fallback + a version preflight block (SELECT extversion)
- identifier casing: embed quotes 'public."SourceEvidence"' in every
partman call and the retention part_config UPDATE (unquoted folds to
lowercase and silently no-ops the provenance guard)
- backfill exit: CALL does not set ROW_COUNT; loop on count(*) of
SourceEvidence_old instead
- rollback: replace ON CONFLICT ("id") (errors under the new
("id","createdAt") PK) with plain INSERT..SELECT back into _old
- import-sweep guard: abort on recent inserts + note the RENAME is the one
genuinely-blocking step; pause the sweep first
Drafts only; not applied.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Files touched
M ops/phase2-drafts/20260901000000_partition_sourceevidence_by_month/migration.sqlM ops/phase2-drafts/20260901000100_sourceevidence_retention_policy/migration.sqlM ops/phase2-drafts/README.md
Diff
commit ddb43e6722cffddda8a262a430e391f6b2f4e0bb
Author: Steve <steve@designerwallcoverings.com>
Date: Mon Aug 31 12:14:11 2026 -0700
TK-10878 Phase 2 drafts: fix 3 fatal + 2 hardening review findings
- p_type: 'native' is invalid; default to 'range' (v5) with flagged v4
'partman' fallback + a version preflight block (SELECT extversion)
- identifier casing: embed quotes 'public."SourceEvidence"' in every
partman call and the retention part_config UPDATE (unquoted folds to
lowercase and silently no-ops the provenance guard)
- backfill exit: CALL does not set ROW_COUNT; loop on count(*) of
SourceEvidence_old instead
- rollback: replace ON CONFLICT ("id") (errors under the new
("id","createdAt") PK) with plain INSERT..SELECT back into _old
- import-sweep guard: abort on recent inserts + note the RENAME is the one
genuinely-blocking step; pause the sweep first
Drafts only; not applied.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
.../migration.sql | 116 +++++++++++++++++----
.../migration.sql | 18 +++-
ops/phase2-drafts/README.md | 27 +++++
3 files changed, 137 insertions(+), 24 deletions(-)
diff --git a/ops/phase2-drafts/20260901000000_partition_sourceevidence_by_month/migration.sql b/ops/phase2-drafts/20260901000000_partition_sourceevidence_by_month/migration.sql
index e5ea741b..ccd9016d 100644
--- a/ops/phase2-drafts/20260901000000_partition_sourceevidence_by_month/migration.sql
+++ b/ops/phase2-drafts/20260901000000_partition_sourceevidence_by_month/migration.sql
@@ -26,7 +26,28 @@
-- ============================================================================
-- ---------------------------------------------------------------------------
--- 0. Guards
+-- 0. PREFLIGHT — DO NOT RUN BLIND. Two facts must be established by the
+-- operator BEFORE executing anything below, because both change what runs.
+--
+-- (A) pg_partman VERSION -> the create_parent p_type value (step 3).
+-- Run this and read the result:
+-- SELECT extversion FROM pg_extension WHERE extname = 'pg_partman';
+-- Then set p_type in step 3 accordingly:
+-- * extversion 5.x (e.g. '5.1.0') -> p_type => 'range' (DEFAULT below)
+-- * extversion 4.x (e.g. '4.7.4') -> p_type => 'partman'
+-- NOTE: 'native' is NOT and never was a valid pg_partman p_type — a blind
+-- run with 'native' fails. This migration DEFAULTS to the v5 value
+-- ('range'); if prod is still on partman 4.x, change the single flagged
+-- line in step 3 to 'partman' before running.
+--
+-- (B) IMPORT SWEEP must be PAUSED. Step 1's RENAME takes ACCESS EXCLUSIVE
+-- and will queue every concurrent writer behind it. Pause the import
+-- launchd sweep first, then confirm the recent-insert guard below reads
+-- near-zero, THEN proceed.
+-- ---------------------------------------------------------------------------
+
+-- ---------------------------------------------------------------------------
+-- 0a. Guards
-- ---------------------------------------------------------------------------
CREATE EXTENSION IF NOT EXISTS pg_partman; -- installs into schema "partman" by default; adjust if your prod uses a different target schema.
@@ -39,9 +60,33 @@ BEGIN
END IF;
END $$;
+-- IMPORT-SWEEP GUARD — abort if the table is still being actively written.
+-- The RENAME in step 1 blocks all writers under ACCESS EXCLUSIVE; running it
+-- while the import sweep is live queues those inserts behind our lock (and, if
+-- lock_timeout fires, aborts mid-flight). If any rows landed in the last
+-- 2 minutes above the threshold, the sweep is presumed live -> STOP, pause it,
+-- and re-run. Tune the window/threshold to your import cadence.
+DO $$
+DECLARE recent bigint;
+BEGIN
+ SELECT count(*) INTO recent
+ FROM "SourceEvidence"
+ WHERE "createdAt" > now() - interval '2 minutes';
+ IF recent > 10 THEN
+ RAISE EXCEPTION
+ 'Import sweep appears LIVE: % rows inserted in the last 2 minutes. '
+ 'Pause the import launchd sweep before running this migration.', recent;
+ END IF;
+END $$;
+
-- ---------------------------------------------------------------------------
-- 1. Rename the live table out of the way; it becomes the source we drain from.
--- A brief ACCESS EXCLUSIVE on the rename only — bounded by lock_timeout.
+-- >>> THIS RENAME IS THE ONE GENUINELY-BLOCKING STEP. <<<
+-- It takes ACCESS EXCLUSIVE and, under an active import sweep, will QUEUE
+-- every writer behind it (or abort when lock_timeout fires). The
+-- "non-blocking / throttled" claim for this migration applies only to the
+-- BACKFILL (step 4) — NOT to this rename. The step-0 import-sweep guard
+-- above must have passed, and the sweep must be paused, before this runs.
-- ---------------------------------------------------------------------------
SET lock_timeout = '5s';
ALTER TABLE "SourceEvidence" RENAME TO "SourceEvidence_old";
@@ -99,10 +144,14 @@ ALTER TABLE "SourceEvidence"
-- SELECT date_trunc('month', min("createdAt")) FROM "SourceEvidence_old";
-- p_premake pre-creates 4 future months so live inserts never hit a gap.
-- ---------------------------------------------------------------------------
+-- IDENTIFIER CASING: the table is created quoted mixed-case ("SourceEvidence"),
+-- so the partman string param MUST embed the quotes. Unquoted 'public.SourceEvidence'
+-- folds to lowercase (public.sourceevidence) and silently targets a table that
+-- does not exist -> create_parent fails / no-ops. Keep the embedded quotes.
SELECT partman.create_parent(
- p_parent_table => 'public.SourceEvidence',
+ p_parent_table => 'public."SourceEvidence"',
p_control => 'createdAt',
- p_type => 'native',
+ p_type => 'range', -- << pg_partman v5.x. If prod is partman 4.x, change to 'partman' (see step-0 preflight). 'native' is INVALID. >>
p_interval => 'monthly',
p_premake => 4,
p_start_partition => '2025-01-01' -- << REPLACE with date_trunc('month', min createdAt) from _old >>
@@ -120,19 +169,29 @@ SELECT partman.create_parent(
-- single migration transaction cannot hold the whole 55 GB move.
--
-- >>> This step is a BABYSAT LOOP, not a fire-and-forget statement. <<<
+--
+-- EXIT CONDITION — CANNOT use GET DIAGNOSTICS ... = ROW_COUNT here. ROW_COUNT
+-- is NOT set by a CALL (procedure invocation), so it reads 0 after the first
+-- pass and the loop exits before the table is drained. Instead, poll the
+-- real remaining count in the SOURCE table between passes and loop until it
+-- hits 0. (Also note the embedded-quote table name — partman needs the
+-- mixed-case identifier quoted, same as create_parent.)
+--
-- Reference loop (psql, run manually during the window):
--
-- DO $$
--- DECLARE moved bigint; total bigint := 0;
+-- DECLARE remaining bigint;
-- BEGIN
-- LOOP
-- CALL partman.partition_data_proc(
--- p_parent_table => 'public.SourceEvidence',
+-- p_parent_table => 'public."SourceEvidence"',
-- p_loop_count => 20, -- intervals per pass
-- p_wait => 2 -- seconds between commits (throttle WAL/disk)
-- );
--- GET DIAGNOSTICS moved = ROW_COUNT; -- 0 when drained
--- EXIT WHEN moved = 0;
+-- -- partition_data_proc DELETEs from _old as it copies, so the source
+-- -- count is the true drain signal. 0 => done.
+-- SELECT count(*) INTO remaining FROM "SourceEvidence_old";
+-- EXIT WHEN remaining = 0;
-- END LOOP;
-- END $$;
--
@@ -145,8 +204,9 @@ SELECT partman.create_parent(
-- 5. CUTOVER VERIFY — do NOT drop _old until BOTH counts match.
-- SELECT count(*) FROM "SourceEvidence"; -- new partitioned parent
-- SELECT count(*) FROM "SourceEvidence_old"; -- MUST be 0
--- And spot-check the partition set is contiguous with no gap at "now":
--- SELECT partman.check_default('public.SourceEvidence');
+-- And spot-check the partition set is contiguous with no gap at "now"
+-- (embedded-quote table name, same as create_parent):
+-- SELECT partman.check_default('public."SourceEvidence"');
-- ---------------------------------------------------------------------------
-- ---------------------------------------------------------------------------
@@ -161,14 +221,24 @@ SELECT partman.create_parent(
-- BEFORE the drop in step 6 (i.e. any time during backfill), abort is trivial
-- and lossless because "SourceEvidence_old" still holds every original row:
--
--- -- 1. Move any rows that already landed in the new parent back is NOT needed
--- -- if you abort before deleting from _old; partition_data_proc DELETEs
--- -- from _old as it copies, so rows are split between the two tables.
--- -- Simplest safe abort = re-drain the new parent back into _old, OR:
+-- -- 1. partition_data_proc DELETEs from _old as it copies, so at abort time
+-- -- rows are SPLIT: the not-yet-moved rows are still in _old, and the moved
+-- -- rows are in the new partitioned parent. To make _old whole again, copy
+-- -- the moved rows back into _old.
-- -- a) SET lock_timeout='5s';
--- -- b) INSERT INTO "SourceEvidence_old"
--- -- SELECT * FROM "SourceEvidence"
--- -- ON CONFLICT ("id") DO NOTHING; -- restore any migrated rows
+-- -- b) -- Restore the migrated rows to _old. "SourceEvidence_old" retains
+-- -- -- the ORIGINAL narrow PK ("id"), and partition_data_proc never
+-- -- -- leaves a duplicate id across the two tables, so a plain
+-- -- -- INSERT ... SELECT is correct AND avoids the ON CONFLICT trap:
+-- -- -- the new parent's PK is ("id","createdAt"), so ON CONFLICT ("id")
+-- -- -- would ERROR (no unique constraint on "id" alone).
+-- -- INSERT INTO "SourceEvidence_old"
+-- -- SELECT * FROM "SourceEvidence"; -- restore any migrated rows
+-- -- -- (If you are unsure whether a pass double-copied, use instead:
+-- -- -- INSERT INTO "SourceEvidence_old"
+-- -- -- SELECT s.* FROM "SourceEvidence" s
+-- -- -- WHERE NOT EXISTS (SELECT 1 FROM "SourceEvidence_old" o
+-- -- -- WHERE o."id" = s."id"); )
-- -- c) DROP TABLE "SourceEvidence" CASCADE; -- the partitioned parent + children
-- -- d) SELECT partman.undo_partition(...) is unnecessary since we drop.
-- -- e) ALTER TABLE "SourceEvidence_old" RENAME TO "SourceEvidence";
@@ -176,16 +246,20 @@ SELECT partman.create_parent(
-- -- ALTER INDEX "SourceEvidence_old_entityType_entityId_idx" RENAME TO "SourceEvidence_entityType_entityId_idx";
-- -- ALTER INDEX "SourceEvidence_old_stagedRecordId_idx" RENAME TO "SourceEvidence_stagedRecordId_idx";
-- -- ALTER TABLE "SourceEvidence" RENAME CONSTRAINT "SourceEvidence_old_stagedRecordId_fkey" TO "SourceEvidence_stagedRecordId_fkey";
--- -- f) partman.part_config row for this parent: DELETE where parent_table='public.SourceEvidence'.
+-- -- f) partman.part_config row for this parent: DELETE where
+-- -- parent_table='public."SourceEvidence"'. (Must match the embedded-quote
+-- -- value create_parent stored — unquoted 'public.SourceEvidence' will NOT
+-- -- match the row and the config would be orphaned.)
--
-- AFTER step 6 (source dropped): rollback is a RESTORE-FROM-BACKUP, which is
-- why the preflight requires a fresh, verified pg_dump/base-backup and why
-- step 6 must not run until step 5 passes. There is no in-place undo once the
-- original rows are gone.
--
--- Partman's own reverse tool (if you keep _old around) :
+-- Partman's own reverse tool (if you keep _old around) — embedded-quote table
+-- name, same as create_parent:
-- SELECT partman.undo_partition(
--- p_parent_table => 'public.SourceEvidence',
--- p_target_table => 'public.SourceEvidence_unpart', -- collapses children back
+-- p_parent_table => 'public."SourceEvidence"',
+-- p_target_table => 'public."SourceEvidence_unpart"', -- collapses children back
-- p_keep_table => false);
-- ============================================================================
diff --git a/ops/phase2-drafts/20260901000100_sourceevidence_retention_policy/migration.sql b/ops/phase2-drafts/20260901000100_sourceevidence_retention_policy/migration.sql
index 990dcf80..63831966 100644
--- a/ops/phase2-drafts/20260901000100_sourceevidence_retention_policy/migration.sql
+++ b/ops/phase2-drafts/20260901000100_sourceevidence_retention_policy/migration.sql
@@ -38,13 +38,23 @@
-- 1. SourceEvidence — explicitly assert NO auto-retention (belt-and-suspenders).
-- part_config is created by create_parent(); we make the intent durable so a
-- future run_maintenance() can never age these out.
+--
+-- CRITICAL — IDENTIFIER CASING: create_parent stored parent_table with the
+-- embedded quotes ('public."SourceEvidence"') because the table is mixed-case.
+-- This WHERE clause MUST match that exact value. The unquoted
+-- 'public.SourceEvidence' matches ZERO rows -> this UPDATE silently affects 0
+-- rows -> the provenance-protection guard is a no-op and auto-retention is
+-- NOT actually disabled. Keep the embedded quotes. Verify after running:
+-- -- expect 1 row, retention = NULL:
+-- SELECT parent_table, retention FROM partman.part_config
+-- WHERE parent_table = 'public."SourceEvidence"';
-- ---------------------------------------------------------------------------
UPDATE partman.part_config
SET retention = NULL, -- no age threshold
retention_keep_table = true, -- if ever set, DETACH (keep table), never drop
retention_keep_index = true,
infinite_time_partitions = true -- always pre-make future months; never stop
- WHERE parent_table = 'public.SourceEvidence';
+ WHERE parent_table = 'public."SourceEvidence"';
-- Provenance guardrail comment for the next operator.
COMMENT ON TABLE "SourceEvidence" IS
@@ -60,11 +70,13 @@ COMMENT ON TABLE "SourceEvidence" IS
-- ONLY on Steve's explicit go, and only after ValidationEvent is partitioned.
--
-- -- Keep ~12 months of validation history, DETACH (not drop) older months:
+-- -- (embedded-quote table name — must match the value create_parent stored;
+-- -- unquoted 'public.ValidationEvent' folds to lowercase and matches 0 rows.)
-- -- UPDATE partman.part_config
-- -- SET retention = '12 months',
-- -- retention_keep_table = true, -- DETACH to a standalone table, do NOT drop
-- -- retention_keep_index = true
--- -- WHERE parent_table = 'public.ValidationEvent';
+-- -- WHERE parent_table = 'public."ValidationEvent"';
-- -- Detached months can then be pg_dump'd to cold storage and dropped by a
-- -- human after review — never by run_maintenance().
-- ---------------------------------------------------------------------------
@@ -93,6 +105,6 @@ COMMENT ON TABLE "SourceEvidence" IS
--
-- ROLLBACK for this migration itself (config change only, no data touched):
-- UPDATE partman.part_config SET retention = <prior value>, ...
--- WHERE parent_table = 'public.SourceEvidence';
+-- WHERE parent_table = 'public."SourceEvidence"'; -- embedded-quote match
-- COMMENT ON TABLE "SourceEvidence" IS NULL;
-- ============================================================================
diff --git a/ops/phase2-drafts/README.md b/ops/phase2-drafts/README.md
index 4e435178..012e0ad4 100644
--- a/ops/phase2-drafts/README.md
+++ b/ops/phase2-drafts/README.md
@@ -5,6 +5,33 @@ These are **draft raw-SQL migrations**, not yet applied. They convert
provenance-safe retention policy. **Run only in a maintenance window on the
grown disk (Phase 1 first).**
+## Operator preflight — DO THESE BEFORE RUNNING (do not run blind)
+
+1. **Establish the pg_partman version — it changes `create_parent`'s `p_type`.**
+ ```sql
+ SELECT extversion FROM pg_extension WHERE extname = 'pg_partman';
+ ```
+ * `5.x` → the partition migration's `p_type => 'range'` (the DEFAULT — no edit needed).
+ * `4.x` → change the single flagged `p_type` line in step 3 to `'partman'`.
+ * `'native'` is **not** and never was a valid pg_partman `p_type`; a blind
+ run with it fails. The draft defaults to `'range'` so it can't run blind.
+
+2. **Pause the import launchd sweep first.** Step 1's `ALTER TABLE … RENAME`
+ takes `ACCESS EXCLUSIVE` — it is the **one genuinely-blocking step** (the
+ "throttled / non-blocking" claim applies only to the step-4 backfill). Under
+ an active import it queues every writer behind the lock (or aborts when
+ `lock_timeout` fires). The migration has a step-0 guard that ABORTS if it
+ sees recent inserts, but pause the sweep explicitly rather than relying on it.
+
+3. **Mixed-case identifier — quotes are load-bearing.** `"SourceEvidence"` is
+ created quoted mixed-case, so every partman call embeds the quotes
+ (`'public."SourceEvidence"'`) in `create_parent`, `partition_data_proc`,
+ `check_default`, and the retention `UPDATE partman.part_config`. The unquoted
+ form folds to lowercase, targets a non-existent table / matches zero
+ `part_config` rows, and — in the retention migration — would silently
+ **no-op the provenance-protection guard** (auto-drop left enabled). Do not
+ un-quote these.
+
## Files
- `20260901000000_partition_sourceevidence_by_month/migration.sql` — pg_partman
conversion. Partition key `createdAt`, monthly. Rename-old → new partitioned
← 21b9aad6 docs(homesonspec): TK-10878 Phase 2 drafts — pg_partman Sour
·
back to Homesonspec
·
TK-10878: monthly SourceEvidence VACUUM/REINDEX maintenance b9a44831 →