From 347ce88d5dd8bc631c073f1947fea6b75d5a43c0 Mon Sep 17 00:00:00 2001 From: Ian Oberst Date: Wed, 21 Oct 2026 17:14:22 -0700 Subject: [PATCH] fix(db): keep pgschema re-apply from dropping runtime partitions (#8487) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary pgschema doesn't compare partition bounds. Re-applying `schema/schema.sql` to a database a relay has used therefore plans `DROP TABLE IF EXISTS ... CASCADE` for every `events` / `delivery_log` monthly the partition manager created at runtime, and those tables' rows are deleted with them. Since #8464 the manager advances the catch-all by default, so any local dev or test database a relay has started against now hits this. One example is the persistent `buzz` database that `scripts/start-relay-for-tests.sh` re-applies. This adds a root `.pgschemaignore` that hides dated monthlies (`events_p2*`, `delivery_log_p2*`) from pgschema. The monthlies declared in `schema/partitions/` are exempted with `!` patterns, so pgschema still manages them. pgschema reads the file from its working directory, and every caller already runs it from the repo root: the test-relay launchers, `postgres-test-setup.sh`, the desktop release smoke and `_ci-relay.yml`. Both partition files now carry a note pointing at it, and AGENTS.md gotcha #7 adds "run pgschema from the repo root" next to the reconcile rule. New-install layout is unchanged. **Why the drops haven't deleted data yet:** on a reconciled database a re-apply also plans to drop and re-add the `events` primary key on the parent and on each declared child. The apply then fails with `cannot drop inherited constraint "events_p2026_01_pkey"` (42P16). That happens with or without this PR. The planned drops run in the same single transaction, so they roll back, and the runtime monthlies have survived so far. Once that primary-key drift is fixed, re-applies will succeed and this ignore file is what keeps them from dropping the runtime monthlies. The primary-key fix is a separate follow-up. ### Related issue Follow-up to #8464. Salvaged from an earlier branch; no existing issue found. ### Testing - `pgschema_reapply_keeps_runtime_partitions` (Postgres) follows the real bootstrap path in a dedicated database: - `bin/pgschema apply` from the repo root, then `scripts/reconcile-schema-after-pgschema.sql`; - the relay's default maintenance pass (create plus advance); - asserts that both catch-alls were attached before the pass and that their bounds moved after it; - asserts that each parent's `p2026_07` monthly exists. The catch-all advanced past the declared bounds but kept its name. - asserts that `pgschema plan` contains no `DROP TABLE` and no line mentioning any runtime monthly. - Falsified both ways: - With `.pgschemaignore` removed, the test fails and prints the planned drops. - Without the reconcile step, it fails with `reconcile did not attach both catch-alls`. - `pgschemaignore_exempts_exactly_the_declared_monthlies` checks that the `!` exemptions exactly match the dated monthlies in `schema/partitions/`. - The admin parity test now shares the `pgschema` helper, so it also runs from the repo root like the real callers. - All `runtime::migration` and `store::partition` tests pass against local Postgres after merging current main, including the ignored ones: 104 passed, run with `--test-threads=1` because the full nextest Postgres lane needs local `dropdb`. `clippy -D warnings` and the pre-push lanes pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Ian Oberst Co-authored-by: Ian Oberst Co-authored-by: Claude Opus 5.5 --- .pgschemaignore | 25 +++ AGENTS.md | 2 +- crates/buzz-db/src/runtime/migration.rs | 252 ++++++++++++++++++++---- schema/partitions/delivery_log.sql | 4 + schema/partitions/events.sql | 4 + 5 files changed, 250 insertions(+), 37 deletions(-) create mode 100644 .pgschemaignore diff --git a/.pgschemaignore b/.pgschemaignore new file mode 100644 index 0000000000..059350e198 --- /dev/null +++ b/.pgschemaignore @@ -0,0 +1,25 @@ +# pgschema reads this file from its working directory; every caller runs it +# from the repository root. +# +# The relay's partition manager creates dated `events` / `delivery_log` +# monthlies at runtime (see crates/buzz-db/src/store/partition/). They are not +# declared in schema/partitions/, so without this file re-applying pgschema to a +# database a relay has used drops them, and their rows, with +# `DROP TABLE IF EXISTS ... CASCADE`. Only the monthlies schema/partitions/ +# declares stay managed by pgschema; keep the `!` exemptions below in sync with +# them. +[tables] +patterns = [ + "events_p2*", + "!events_p2026_01", + "!events_p2026_02", + "!events_p2026_03", + "!events_p2026_04", + "!events_p2026_05", + "!events_p2026_06", + "delivery_log_p2*", + "!delivery_log_p2026_03", + "!delivery_log_p2026_04", + "!delivery_log_p2026_05", + "!delivery_log_p2026_06", +] diff --git a/AGENTS.md b/AGENTS.md index c2aba29217..665ac30312 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -613,7 +613,7 @@ description. See [PR #803](https://github.com/block/buzz/pull/803). 4. **Worktrees: `cd` in the same command** — shell CWD doesn't persist between tool calls. Use `cd /path && cargo build` as one command. 5. **Desktop crate excluded from root workspace** — `cargo test` at repo root does NOT run desktop tests. Use `cargo test --manifest-path desktop/src-tauri/Cargo.toml` explicitly. 6. **React render perf: `React.memo` is all-or-nothing** — it only skips a re-render when *every* prop is reference-stable; one unstable prop (inline arrow/JSX, or a hook returning a fresh `{}`/`[]`/`Map` each render) defeats it. Two repeat offenders: (a) React Query results (`useMutation`/`useQuery`) are a **new object each render** — depend on the stable method (`mutation.mutateAsync`), not the object; (b) derived `Map`/array state that recomputes on a version bump — wrap in a content-equality ref cache (`shared/hooks/useStableReference.ts`). When chasing interaction lag, **measure with DevTools closed and no perf probes** (an open Web Inspector + per-keystroke `console.log` inflate the numbers), and isolate by removing one suspect at a time rather than guessing. -7. **`pgschema` omits seed DML and some storage parameters** — `schema/schema.sql` is an ordered `\i` manifest: `schema/tables/public/.sql` holds only that table's `CREATE TABLE` and `CREATE INDEX` statements (one table per file, the shape out-of-band schema tools read), and types, functions, partitions, triggers, and seed rows live in the sibling `schema/` directories. Fresh desired-state bootstraps use `./bin/pgschema apply`, which does not execute `INSERT` statements or preserve every table storage parameter from `schema/schema.sql`. Put each unsupported invariant in `scripts/reconcile-schema-after-pgschema.sql` as an idempotent convergence statement plus a live catalog or data assertion. Every `pgschema apply` caller must run that script. A string assertion against `schema.sql` alone does not prove the pgschema-created database has the intended state. +7. **`pgschema` omits seed DML and some storage parameters** — `schema/schema.sql` is an ordered `\i` manifest: `schema/tables/public/
.sql` holds only that table's `CREATE TABLE` and `CREATE INDEX` statements (one table per file, the shape out-of-band schema tools read), and types, functions, partitions, triggers, and seed rows live in the sibling `schema/` directories. Fresh desired-state bootstraps use `./bin/pgschema apply`, which does not execute `INSERT` statements or preserve every table storage parameter from `schema/schema.sql`. Put each unsupported invariant in `scripts/reconcile-schema-after-pgschema.sql` as an idempotent convergence statement plus a live catalog or data assertion. Every `pgschema apply` caller must run that script, and must run pgschema from the repository root so it reads `.pgschemaignore`; from any other directory a re-apply plans to drop the partitions the relay created at runtime. A string assertion against `schema.sql` alone does not prove the pgschema-created database has the intended state. --- diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index d194f0fa35..af87733252 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -236,6 +236,66 @@ mod postgres_tests { .unwrap_or_default() } + /// Runs `bin/pgschema ` against `db`, which doubles as the plan + /// database (`--plan-*`): that avoids the embedded-Postgres download and + /// matches start-relay-for-tests.sh. Runs from the repository root, like + /// every pgschema caller, so `.pgschemaignore` applies. + fn pgschema(conn: &PgConn, db: &str, command: &[&str]) -> std::process::Output { + let repo_root = Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."); + let port = conn.port.to_string(); + std::process::Command::new(repo_root.join("bin/pgschema")) + .current_dir(&repo_root) + .args(command) + .args([ + "--file", + "schema/schema.sql", + "--host", + &conn.host, + "--port", + &port, + "--user", + &conn.user, + "--password", + &conn.password, + "--db", + db, + "--plan-host", + &conn.host, + "--plan-port", + &port, + "--plan-user", + &conn.user, + "--plan-password", + &conn.password, + "--plan-db", + db, + ]) + .output() + .expect("run bin/pgschema (hermit env required)") + } + + /// Dated monthlies `schema/partitions/` declares, e.g. `events_p2026_01`. + fn declared_monthlies() -> BTreeSet { + let dir = crate::test_support::desired_state_schema_dir().join("partitions"); + let mut names = BTreeSet::new(); + for entry in fs::read_dir(&dir).expect("read schema/partitions") { + let sql = fs::read_to_string(entry.expect("partition file").path()) + .expect("read partition file"); + names.extend( + sql.lines() + .filter_map(|line| line.strip_prefix("CREATE TABLE ")) + .filter_map(|rest| rest.split_whitespace().next()) + .filter(|name| { + name.rsplit_once("_p").is_some_and(|(_, month)| { + month.starts_with(|c: char| c.is_ascii_digit()) + }) + }) + .map(str::to_owned), + ); + } + names + } + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum ConstraintKind { ForeignKey, @@ -1517,6 +1577,32 @@ mod postgres_tests { ); } + /// `.pgschemaignore` hides dated monthlies from pgschema so a re-apply + /// cannot drop the ones the relay created at runtime. Each monthly + /// `schema/partitions/` declares must be exempt, or pgschema would stop + /// creating it on a fresh database. + #[test] + fn pgschemaignore_exempts_exactly_the_declared_monthlies() { + let ignore = + fs::read_to_string(Path::new(env!("CARGO_MANIFEST_DIR")).join("../../.pgschemaignore")) + .expect("read .pgschemaignore"); + let exempt: BTreeSet = ignore + .lines() + .filter_map(|line| { + line.trim() + .trim_end_matches(',') + .strip_prefix("\"!")? + .strip_suffix('"') + }) + .map(str::to_owned) + .collect(); + assert_eq!( + exempt, + declared_monthlies(), + ".pgschemaignore `!` exemptions must match the monthlies schema/partitions/ declares" + ); + } + #[test] fn workflow_run_error_codes_are_additive_and_backfilled_without_parsing_diagnostics() { let mut migrations: Vec<_> = MIGRATOR.iter().collect(); @@ -2703,42 +2789,8 @@ mod postgres_tests { .expect("create migrated probe database"); // Bootstrap the desired-state probe through the real pgschema binary, the - // same invocation the test-relay launchers use. The freshly-created probe - // db doubles as pgschema's plan database (--plan-*), which avoids the - // embedded-Postgres download and matches start-relay-for-tests.sh. - let pgschema = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../bin/pgschema"); - let schema_file = - std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../schema/schema.sql"); - let port = conn.port.to_string(); - let apply = std::process::Command::new(&pgschema) - .args([ - "apply", - "--auto-approve", - "--file", - schema_file.to_str().expect("schema path utf-8"), - "--host", - &conn.host, - "--port", - &port, - "--user", - &conn.user, - "--password", - &conn.password, - "--db", - &desired_db, - "--plan-host", - &conn.host, - "--plan-port", - &port, - "--plan-user", - &conn.user, - "--plan-password", - &conn.password, - "--plan-db", - &desired_db, - ]) - .output() - .expect("run bin/pgschema apply (hermit env required)"); + // same invocation the test-relay launchers use. + let apply = pgschema(&conn, &desired_db, &["apply", "--auto-approve"]); assert!( apply.status.success(), "pgschema apply failed: {}\n{}", @@ -2790,6 +2842,134 @@ mod postgres_tests { } } + /// Re-applying `schema/schema.sql` to a database a relay has used plans + /// `DROP TABLE IF EXISTS ... CASCADE` for every monthly the partition + /// manager created at runtime, and their rows go with them. + /// `.pgschemaignore` keeps those monthlies out of pgschema's view, and + /// pgschema does not compare partition bounds, so the advanced catch-all + /// is left alone too. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn pgschema_reapply_keeps_runtime_partitions() { + use crate::store::partition::{ + maintain_partitions, PartitionMaintenancePolicy, PARTITION_MANAGER_MONTHS_AHEAD, + }; + use sqlx::AssertSqlSafe; + + let base_url = crate::test_support::database_url(); + let conn = parse_pg_url(&base_url); + let admin = PgPool::connect(&base_url) + .await + .expect("connect admin database"); + let (base_prefix, _) = base_url.rsplit_once('/').expect("database url has a path"); + let database = format!("buzz_pgschema_reapply_{}", uuid::Uuid::new_v4().simple()); + sqlx::query(AssertSqlSafe(format!("CREATE DATABASE {database}"))) + .execute(&admin) + .await + .expect("create probe database"); + + let apply = pgschema(&conn, &database, &["apply", "--auto-approve"]); + assert!( + apply.status.success(), + "pgschema apply failed: {}\n{}", + String::from_utf8_lossy(&apply.stdout), + String::from_utf8_lossy(&apply.stderr), + ); + + let pool = PgPool::connect(&format!("{base_prefix}/{database}")) + .await + .expect("connect probe database"); + // Every pgschema caller attaches the declared children this way. + sqlx::raw_sql(include_str!( + "../../../../scripts/reconcile-schema-after-pgschema.sql" + )) + .execute(&pool) + .await + .expect("reconcile schema after pgschema"); + async fn catch_all_bounds(pool: &PgPool) -> Vec<(String, Option)> { + sqlx::query_as( + "SELECT relname::text, pg_get_expr(relpartbound, oid) FROM pg_class \ + WHERE relname IN ('events_p_future', 'delivery_log_p_future') \ + ORDER BY relname", + ) + .fetch_all(pool) + .await + .expect("read catch-all bounds") + } + let declared_bounds = catch_all_bounds(&pool).await; + assert!( + declared_bounds.len() == 2 && declared_bounds.iter().all(|(_, bound)| bound.is_some()), + "reconcile did not attach both catch-alls: {declared_bounds:?}" + ); + + // The relay's default startup pass advances each catch-all into dated + // monthlies schema/partitions/ does not declare. + maintain_partitions( + &pool, + PARTITION_MANAGER_MONTHS_AHEAD, + PartitionMaintenancePolicy { + create_enabled: true, + advance_enabled: true, + }, + ) + .await + .expect("maintain partitions"); + let advanced_bounds = catch_all_bounds(&pool).await; + for (declared, advanced) in declared_bounds.iter().zip(&advanced_bounds) { + assert_ne!(declared, advanced, "catch-all did not advance"); + } + let children: Vec<(String, String)> = sqlx::query_as( + "SELECT p.relname::text, c.relname::text FROM pg_inherits i \ + JOIN pg_class c ON c.oid = i.inhrelid \ + JOIN pg_class p ON p.oid = i.inhparent \ + WHERE p.relname IN ('events', 'delivery_log')", + ) + .fetch_all(&pool) + .await + .expect("list partitions"); + pool.close().await; + let declared = declared_monthlies(); + let runtime: Vec<_> = children + .iter() + .filter(|(parent, child)| { + child.starts_with(&format!("{parent}_p2")) && !declared.contains(child) + }) + .collect(); + for parent in ["events", "delivery_log"] { + // The first month the declared catch-all served. + let first = format!("{parent}_p2026_07"); + assert!( + runtime.iter().any(|(_, child)| *child == first), + "maintenance did not advance into {first}: {children:?}" + ); + } + + let plan = pgschema(&conn, &database, &["plan", "--output-sql", "stdout"]); + let plan_sql = String::from_utf8_lossy(&plan.stdout); + assert!( + plan.status.success(), + "pgschema plan failed: {plan_sql}\n{}", + String::from_utf8_lossy(&plan.stderr), + ); + assert!( + !plan_sql.contains("DROP TABLE"), + "re-applying schema.sql would drop tables:\n{plan_sql}" + ); + for (_, child) in &runtime { + assert!( + !plan_sql.contains(child.as_str()), + "re-applying schema.sql would change runtime monthly {child}:\n{plan_sql}" + ); + } + + sqlx::query(AssertSqlSafe(format!( + "DROP DATABASE {database} WITH (FORCE)" + ))) + .execute(&admin) + .await + .expect("drop probe database"); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn pre_0007_ambiguous_nip_rs_data_blocks_without_mutation_and_allows_retry() { diff --git a/schema/partitions/delivery_log.sql b/schema/partitions/delivery_log.sql index 9314b0b4b8..f37c086585 100644 --- a/schema/partitions/delivery_log.sql +++ b/schema/partitions/delivery_log.sql @@ -1,3 +1,7 @@ +-- The static layout migrations/0001 creates. The relay's partition manager +-- (crates/buzz-db/src/store/partition/) adds later monthlies at runtime; +-- .pgschemaignore hides those from pgschema and must exempt every dated +-- monthly declared here. CREATE TABLE delivery_log_p_past PARTITION OF delivery_log FOR VALUES FROM (MINVALUE) TO ('2026-03-01'); CREATE TABLE delivery_log_p2026_03 PARTITION OF delivery_log diff --git a/schema/partitions/events.sql b/schema/partitions/events.sql index ffb257390c..e5c9ef75d4 100644 --- a/schema/partitions/events.sql +++ b/schema/partitions/events.sql @@ -1,3 +1,7 @@ +-- The static layout migrations/0001 creates. The relay's partition manager +-- (crates/buzz-db/src/store/partition/) adds later monthlies at runtime; +-- .pgschemaignore hides those from pgschema and must exempt every dated +-- monthly declared here. CREATE TABLE events_p_past PARTITION OF events FOR VALUES FROM (MINVALUE) TO ('2026-01-01'); CREATE TABLE events_p2026_01 PARTITION OF events