mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
fix(v24): guard budget_ledger + budget_reservations with IF EXISTS
Garry flagged: migration v24 fires `ALTER TABLE budget_ledger ENABLE
ROW LEVEL SECURITY` unconditionally. budget_ledger and
budget_reservations are migration-only (v12) — not in schema.sql,
not re-created on every initSchema. In the normal flow v12 runs
before v24 so they exist, but two edge cases break that assumption:
1. An operator manually dropped them (budget data is regenerable
from resolver call logs, so `DROP TABLE` is a reasonable
cleanup move).
2. A brain was somehow running an old gbrain that lacked v12, and
is only catching up now.
Bare ALTER hits 42P01 (relation does not exist), aborts the
transaction, and leaves schema_version at 23. On next initSchema,
v24 retries and hits the same error — stuck in a loop.
Fix: wrap each of the two budget ALTERs in
IF EXISTS (SELECT 1 FROM information_schema.tables
WHERE table_schema = 'public'
AND table_name = '<tbl>') THEN ... END IF;
The other 8 tables are not guarded. schema.sql creates them
idempotently on every initSchema run before migrations fire, so
they are guaranteed to exist by the time v24 runs. Adding guards
there would be unnecessary and make the SQL noisier.
Also simplified the DECLARE/BEGIN structure: moved the
non-BYPASSRLS early-exit to the top so the happy path reads
cleanly without the outer IF.
Tests:
- test/migrate.test.ts: new assertion that both budget_* ALTERs
are wrapped in information_schema.tables IF EXISTS blocks;
BYPASSRLS gate assertion relaxed to match either phrasing.
- Manual e2e: fresh Postgres init (v0→v24), then DROP TABLE
budget_ledger + budget_reservations, reset version=23, re-run
init. v24 applied cleanly, version advanced to 24, budget_*
stayed dropped. Without the guard this would have errored out.
This commit is contained in:
+22
-1
@@ -218,7 +218,28 @@ describe('migration v24 — rls_backfill_missing_tables', () => {
|
||||
const v24 = MIGRATIONS.find(m => m.version === 24);
|
||||
const sql = v24!.sql || '';
|
||||
expect(sql).toContain('rolbypassrls');
|
||||
expect(sql).toContain('IF has_bypass THEN');
|
||||
// The gate can be either IF has_bypass / early-raise pattern.
|
||||
expect(sql).toMatch(/IF (NOT )?has_bypass/);
|
||||
});
|
||||
|
||||
// Self-healing guard: the budget_* tables are migration-only (v12). If an
|
||||
// operator manually dropped them, or if a brain was somehow pinned to a
|
||||
// pre-v12 version when those tables didn't exist, a bare `ALTER TABLE
|
||||
// budget_ledger ...` would fail with 42P01 and abort v24. Wrapping those
|
||||
// two ALTERs in an `IF EXISTS (information_schema.tables ...)` check lets
|
||||
// the migration skip them silently instead of erroring out. The other 8
|
||||
// tables are created by schema.sql on every initSchema and don't need
|
||||
// the guard — bare ALTER is fine.
|
||||
test('guards budget_ledger + budget_reservations with information_schema.tables IF EXISTS', () => {
|
||||
const v24 = MIGRATIONS.find(m => m.version === 24);
|
||||
const sql = v24!.sql || '';
|
||||
// Both budget tables must be wrapped in an existence check.
|
||||
expect(sql).toMatch(
|
||||
/IF EXISTS \(SELECT 1 FROM information_schema\.tables[\s\S]{0,200}table_name = 'budget_ledger'\)[\s\S]{0,200}ALTER TABLE budget_ledger ENABLE ROW LEVEL SECURITY/,
|
||||
);
|
||||
expect(sql).toMatch(
|
||||
/IF EXISTS \(SELECT 1 FROM information_schema\.tables[\s\S]{0,200}table_name = 'budget_reservations'\)[\s\S]{0,200}ALTER TABLE budget_reservations ENABLE ROW LEVEL SECURITY/,
|
||||
);
|
||||
});
|
||||
|
||||
// Codex found: if v24 RAISE WARNINGs instead of raising on non-BYPASSRLS,
|
||||
|
||||
Reference in New Issue
Block a user