feat(pg-delta)!: clean-room rewrite, promoted to @supabase/pg-delta (BREAKING alpha)#299
feat(pg-delta)!: clean-room rewrite, promoted to @supabase/pg-delta (BREAKING alpha)#299jgoux wants to merge 183 commits into
Conversation
🦋 Changeset detectedLatest commit: 3dbf75c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
commit: |
Records the outcome of a full architecture review of the two core packages: verified findings (extraction snapshot consistency bug, JSON.stringify equality cost, sort-layer scaling, objects/ boilerplate volume, WASM dependency tax), the decisions taken (moderate packaging, shadow-DB declarative convergence), what explicitly stays, and a 7-phase incremental roadmap. Docs-only change: no package behavior change, no changeset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per maintainer direction, the document now defines the technically optimal architecture independent of past choices, with the incremental phases re-derived as steps toward it: - P1: Postgres as the only semantic elaborator (one engine, not three) - P2: PostgreSQL knowledge in two forms (extraction queries + rule table) - Content-addressed fact base; generic hash diff with zero per-type code - Maximal decomposition + cosmetic compaction (cycle avoidance replaces the cycle-breaker repair registry) - Single mixed dependency graph replaces two-phase sort + invalidates - First-class proof loop (apply-to-clone, re-extract, hash-compare) as the correctness oracle, enabling generative testing and rename detection - pg-topo repositioned as dev-experience layer outside the trusted path Supersedes the earlier incremental-roadmap framing (see Decision log). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Following maintainer review of whether the change-based data structures and captured data are the technical optimum: - Fact base specified as fully normalized (every addressable thing — column, constraint, default, ACL entry, comment, label — is its own fact with a parent relation), content-addressed with Merkle rollup hashes for O(changed) diffing - Diff output specified as fact-level deltas (add/remove/set) — makes 'zero per-type diff code' structurally true rather than aspirational - Cross-cutting metadata becomes ordinary target-referencing facts: one global rule per kind replaces the per-object-type scope axis - New §8.7: the three-granularities defect (document equality vs sub-entity dependencies vs statement actions) as the root cause of most hand-written translation code - Rename detection extended to sub-entities (column renames) - Path gains a fact-normalization phase (now 10 phases); decision log updated Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Following maintainer review of whether current tests should survive the greenfield design: - §3.7: proof loop gains a data-preservation check (seeded rows must survive wherever the plan claims dataLoss: none) — closes the convergent-but-destructive blind spot of schema-state proof; the safety report becomes a verified claim - §4.3 rewritten as the test architecture: one proof harness + the existing integration scenarios ported as the seed corpus (the problem corpus of field-discovered failures), generative exploration on top, semantic plan assertions (action kinds/budgets, never SQL bytes) for minimality, and old-vs-new differential testing during migration - Per-class unit tests and byte-level SQL snapshots are explicitly not ported; extraction tests and Supabase policy tests survive unchanged - §5/§6/§9/§10 updated accordingly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pre-handoff additions before implementation planning is delegated: - §3.9: integrations specified as a policy layer over deltas — filtering as predicates over deltas/facts (powered by provenance edges), serialize options as rule parameterization, platform baselines as fact-base subtraction; a vendor integration becomes a versionable data package tested via the proof harness - §10: consolidated implementer guardrails — nine absolute invariants (frozen wire format, no parsers in the trusted path, no rule escape hatches, no cycle breakers, provePlan before emission changes, no byte assertions, non-negotiable gates, one granularity, proof as arbiter) with the amend-the-doc-first rule - Decision log renumbered to §11 with entry (d) marking the document ready for phase-by-phase implementation planning Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All 13 review findings verified against the codebase and addressed: - §3.1: collision-resistant (>=128-bit) identity-free hashing specified; edges fold into rollups; per-kind equality-surface policy (extension version example) - §3.3: Delta gains link/unlink verbs for edge-only changes - §3.2: shadow loader specifics — fail-safe best-effort ordering, restored body validation (mirrors round-apply.ts:445-448), cluster isolation for shared objects (roles), parser-free DML rejection - §3.7: fact-base re-creation as proof materialization for live sources (TEMPLATE needs a connection-free source); safety claims narrowed — data loss proven, rewrite risk observed via relfilenode, lock classes vetted-not-proven - §4.1: rename mechanics — identity-free structural rollups, cross-reference caveat, graceful degradation to drop+create - §4.3: independent extractor ring + pg_dump as outside observer against vacuous proof - §4.5: pgdelta binary ships from the CLI package; north-star declarative path needs no pg-topo - §7: pg_depend routine-body blind-spot strategy (layered, no parsing in trusted path) - §11: decision-log entry (e); the suggested deep-compare-on-match fallback rejected with rationale (wide digest instead) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per maintainer direction (brand-new library, no past decisions carried): - Stable-ID wire format unfrozen: identity is structured end-to-end with a single library-side codec; extraction SQL returns identity parts (kind, schema, name, args, parent) as structured columns and never synthesizes strings — killing the dual TS/SQL format maintenance; persisted artifacts are version-tagged instead of frozen - Policy DSL designed fresh for facts/deltas (evaluation model kept, pattern syntax not carried) - Plan/snapshot formats new and version-tagged from v1 - §5 reframed: 'imported assets' (knowledge as data/SQL), not surviving mechanisms or formats - §9 rewritten: clean-room build-and-cutover plan (10 stages) replaces in-place migration phases; old engine = asset donor + differential oracle only; byte-identical SQL gates removed entirely — proof, differential, and fixture-ring gates everywhere; consumers migrate once at the cutover parity bar - Guardrails 1/5/7 rewritten accordingly; §7 'consumers migrate once' cost replaces the migration-output-drift item; decision-log entry (f) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
One document per stage (stage-00 through stage-10), each covering: goal, deliverables, design details, old-codebase mining maps, ordered steps, pitfalls, and the gate in checkable form. Per maintainer direction, the path gains stage 0 — the test suite is built before any engine code: the scenario corpus ported from the existing integration tests, target API stubs throwing NotImplementedError, and old-engine baselines. Refinement over plain 'all red': two layers, so red always means engine-missing (pinned EXPECTED_RED list) and never fixture-broken (fixture-validity layer is green from day one, proving the corpus DDL applies on every PG version). Notable sequencing details encoded in the stage docs: - render-from-fact-base materialization is honestly deferred from stage 3 (harness) to stage 6 (it requires the planner: plan(empty->fb)) - stage 5 maps every old cycle-breaker to a corpus scenario that must sort cycle-free by construction - stage 7 ports round-retry mechanics as the shadow loader only, never a production apply path Main doc: §9 gains the stage-0 row and links to the stage documents; decision-log entry (g). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…idelity Export was one line in the CLI deliverable; it is now its own deliverable: render via the stage-6 renderer, split files via a mapping policy (mined from the old export/file-mapper.ts), with the gate load(export(fb)) == fb hash-identically — closing the declarative loop in both directions, and exports emitted pre-ordered (zero deferred rounds on reload). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…contract Mutual inline FK clauses converge under no permutation and no retry rounds — the loader must say so explicitly (split one FK into ALTER TABLE ADD CONSTRAINT), with a corpus scenario asserting the diagnostic. States the contract: convergence or loud failure, never silent wrongness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed executor transactional / nonTransactional / commitBoundaryAfter — the third class (ALTER TYPE ADD VALUE: runs in a txn but its effect is unusable until commit) forces a segment boundary before any consumer, derived from the existing dependency edges. Clarifies segmentation moves transaction boundaries only, never order, and adds executor options (lock_timeout/statement_timeout per segment, retry-on-lock-timeout) as operational policy distinct from safety metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two independent audit passes over all 12 documents (consistency + completeness). Consistency fixes: - §3.1 property count (six, not five) - §3.6 tie-break aligned with stage 5 (kind weight -> canonical identity; no phase tier in a one-graph sort) - §9 stage-3 row no longer claims both materialization forms (render- from-fact-base is stage 6); stage-9 row gains export + drift - §8 findings citation note (§8.1-§8.7 shorthand) - stage-00 stub named 'extract' to match the stage-9 public API - stage-07 'four obligations' wording (six steps bracketing the four) - stage-09 dependency header includes stage 7 (export round-trip) - decision-log (e) accepted/rejected wording; rename-policy default removed from open questions (stage 9 decided it) Ownership gaps closed: - drift detection -> stage 9 deliverable + CLI verb + gate - dangling-requirement check -> stage 5 graph build (stage 8 supplies the policy negative test) - vetted lock-class table -> stage 5 (net-new; stage 6 consumes) - >=10k benchmark fixture + timing harness -> stage 5, in CI from then - generator kind-coverage growth -> stage 5 per-PR + stage 10 bar - new package's CI lane -> stage 0 deliverable + gate - plan-artifact version rejection -> stage 6 (mirrors snapshot check) - shared diagnostic type -> stage 1 - supported-PG-versions policy (15/17/18, change = logged product decision) -> §9 - connection/shadow trust-boundary notes -> stages 2 and 7 - CLI mapping completed (sync, catalog-export -> snapshot); stage-08 script paths corrected; stage-05 gains its open-decisions section - decision-log entry (h) records the pass Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ase, snapshot, diff Clean-room package per docs/target-architecture.md. TDD throughout: - StableId codec (structured identity, one escaping rule, recursive wrapper kinds; 30 tests incl. ambiguity + malformed input) - canonical payload encoding + SHA-256 content hashes (golden-pinned) - FactBase with named Merkle rollups (edges folded) and identity-free structural rollups; dangling edges -> diagnostics - snapshot format v1 (version-tagged, digest re-verified on load) - generic rollup-guided diff -> add/remove/set/link/unlink deltas (zero per-kind code; diff(A,A)=[] and mirror properties tested) 60 unit tests green.
…depend edges Single REPEATABLE READ READ ONLY transaction (consistent by construction). Kinds: schema, role, extension, table, column, default (own fact, pg_attrdef model), constraint, index, sequence, view, materializedView, procedure/function (proargtypes identity, prokind), trigger, policy; comments + ACLs as satellite facts. pg_depend edges resolved to structured identity parts in SQL — at column grain, which is exactly the fact model's granularity. Identity-column sequences and constraint-backed indexes excluded; extension members filtered (stage-8 TODO: provenance edges). Fixture ring green (11 tests): payload normalization, canonical defs, edge grain, deterministic re-extraction, snapshot round-trip, and TEMPLATE-clone fidelity.
…nner, executor The proof loop is green end-to-end: 12 corpus scenarios proven in BOTH directions (24/24) — state proof (apply plan to TEMPLATE clone, re-extract, zero drift deltas) plus row-count data preservation. - corpus: 12 scenarios (table/column/constraint/index/view/function/ sequence/fk/comments/rls/trigger/grants) with a fixture-validity layer that is green independently of the engine - rule table (§3.4): the only per-kind logic — create/drop/attribute rules incl. global comment + ACL rules written once - planner (§3.5-3.6): deltas -> atomic actions with cascade suppression mapped onto drop roots, replace-with-satellite-recreate, missing-requirement loud failures; ONE mixed graph (consumes/produces/ destroys + desired-state build edges + source-state teardown edges + parent teardown + replace ordering) sorted by heap Kahn with kind-weight tie-breaks; cycles throw with the path (guardrail 4) - executor (§3.8 v1): single transaction, check_function_bodies=off preamble, per-statement error attribution - key extractor fix found by the proof loop: constraint-backed indexes resolve to their constraint fact in pg_depend edges, so FK -> referenced-PK ordering survives the index-fact exclusion engine 24/24, extractor ring 11/11, units 60/60, tsc clean
- provePlan as a first-class product API (§3.7): state proof + data preservation on a sacrificial clone; the engine suite now consumes it - first delta-set rule (§3.4): a column arriving WITH a default inlines DEFAULT into ADD COLUMN and produces both facts — proven by seeded corpus scenarios (NOT NULL + default onto populated tables) - stage 7 shadow loader: bounded-round fail-safe ordering at file granularity, routine body re-validation with checks on, shared-object (role) leak detection, parser-free DML rejection by observation; full declarative e2e (files -> shadow -> plan -> prove vs live target) - CI lane (stage-0 gate): PG 15/17/18 matrix, typecheck + unit + integration - README with honest coverage table and v1 simplifications - oxlint/oxfmt clean under the repo config PG17: 113/113 (60 unit + 53 integration). PG15: 53/53 integration. rtk tsc clean.
…46/390 proofs green) Engine: domains (+domain constraints via ALTER DOMAIN), enum/composite/ range types (enum growth as ADD VALUE subsequence algorithm), collations (collversion excluded), event triggers, rewrite rules, aggregates (reconstructed from pg_aggregate), partitioned tables (PARTITION OF / PARTITION BY with column inlining, pg_inherits edges, inherited column/index/trigger exclusion), replica identity, FDW/server/user- mapping/foreign-table, publications (object lists, column lists, row filters), subscriptions, memberships, default privileges, role configs. Structural fixes proven by the corpus: - OWNED BY as payload + ALTER SEQUENCE OWNED BY (pg_dump's model) — the auto edge cycled with column defaults; owned sequences cascade- suppress into their owning column/table drops - FK constraint drops are NEVER suppressed into table drops: mutual-FK teardown cycles become unconstructible (decomposition over repair) - forced dependent rebuild: surviving dependents of replaced/dropped facts (views/policies/indexes/...) drop+recreate recursively - releases[] edges: owner alters run before the old owner's DROP ROLE - routine def changes replace (return-type changes) + rebuild dependents - column type changes sandwich DROP DEFAULT / TYPE..USING / SET DEFAULT - extension-member objects resolve to their extension in pg_depend edges (index-on-extension-opclass ordering) - role drops emit DROP OWNED BY first; membership revokes CASCADE - subscription dbs: drop cleanup + clone presync (TEMPLATE skips shared catalogs) Harness: meta.json (isolatedCluster on a fresh cluster pair with role cleanup, minVersion), EXPECTED_RED ledger, fixture validity 195/195. Engine proofs: 346/390, triage continuing.
… update Every old integration file (63 + 2 root) accounted for: ported (with per-case agent ledgers merged in), or not-ported-with-reason (Supabase-image, policy-layer/stage-8, dummy_seclabel image, stage-9 renames/export, old-engine internals).
…RTING.md) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix batch driven by the 12 remaining red corpus proofs (each failed
against the prior engine, captured in the proof-loop output):
- enum value removal/reorder: rename-aside value-set migration (rename,
CREATE TYPE with desired values, per-column ::text cast, drop old) +
rebuildsDependents rule hook seeding the forced-rebuild pass so
views/defaults/routines referencing the type are dropped and recreated
around the migration ('type X already exists' / throw-by-design)
- routines added to REBUILDABLE so signature-referencing functions
rebuild when their types are migrated
- identity columns: payload carries the backing sequence; identity
transitions declare it via alsoDestroys/alsoProduces so serial<->identity
swaps order DROP IDENTITY vs CREATE SEQUENCE correctly ('relation
items_c2_seq already exists')
- destroy-before-reproduce edge hoisted above the source.has() guard so
it also applies to ids that exist only physically (identity sequences)
- ACL extraction coalesces NULL acl columns through acldefault() —
pg_dump's model — so a REVOKE that merely instantiates the owner's
implicit grant is not drift ('remove acl:(mv).test' proof failure)
- planner: missing-requirement only throws when the DESIRED state still
carries the edge; otherwise the dependent's alters order before the
destroyer (ALTER PUBLICATION ... SET delisting a dropped table/column)
- planner: two generic edge rules — dependent teardowns precede in-place
alters of their dependencies, and creates follow alters of their kept
dependencies (guarded against the REPLICA IDENTITY USING INDEX inverse)
- creates emit parents-first so partitioned-parent column inlining
registers before child column adds ('column created_at already exists')
- tie-break key zero-pads the action index (string '10' < '9' scrambled
multi-spec sequences)
- servers parent to the extension when their FDW is extension-owned;
extension-member FDW references resolve to the extension (FactBase
integrity error on postgres_fdw)
- EXPECTED_RED: pin mixed-objects--enum-replace-with-dependents:reverse
(ADD VALUE + same-transaction usage needs execution-context
segmentation, target-architecture §3.7)
- tests: corpus loader gains PGDELTA_NEXT_ONLY / PGDELTA_NEXT_SHARD for
focused and parallel runs
Validation: PG 17 and PG 15 both 390/390 engine proofs + 212/212
extract/loader/fixture-validity; unit 60/60; tsc + oxlint clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…licy, renames, API/CLI) Completes the target-architecture stage docs beyond the engine core: Stage 5 (planner leftovers): - vetted lock-class table (src/plan/locks.ts) — per-(kind,verb) documented lock levels, reported not certified; per-form unit assertions - compaction pass — column clauses fold into CREATE TABLE when no graph edge crosses the merge; cosmetic by contract, proof-stability gate asserts on==off proof results (tests/compaction.test.ts) - benchmark fixture + timing harness (scripts/benchmark.ts, ~11.5k facts) - generative engine + soak (tests/generative.test.ts, PGDELTA_NEXT_SOAK) with a kind-coverage checklist Stage 6 (execution + artifacts): - three-valued transactionality in the rule table (transactional / nonTransactional / commitBoundaryAfter); ALTER TYPE ADD VALUE and CREATE INDEX CONCURRENTLY now declared correctly - segmented executor (src/apply/apply.ts): maximal transaction runs, isolated non-transactional actions, commit boundaries before consumers of a commitBoundaryAfter action; per-action applied/unapplied/inDoubt report; fingerprint gate (re-extract + refuse stale plans) - plan artifact v1 (src/plan/artifact.ts): version-tagged JSON, engineVersion check, lossless round-trip incl. bigint payloads - safetyReport + session preamble as plan metadata - render-from-fact-base materialization proof path - un-pinned the enum EXPECTED_RED entry (now handled by the commit boundary); EXPECTED_RED is empty Stage 8 (policy layer): - policy DSL v2 (src/policy/): typed serializable predicates over fact kind/identity/provenance/verbs, first-match-wins, extends with cycle detection; filterDeltas reports filtered deltas (never silent); serialize params declared by the rule table; baseline subtraction - Supabase policy package ported from the old integration (rule-by-rule mapping table); skipAuthorization/skipSchema serialize params wired per-fact in the planner Stage 9 (renames + API/CLI): - rename detection over structural rollups (auto/prompt/off), leaf + container renames, ambiguity/near-miss verdicts, swaps unconstructible by design; data preservation proven to column-value granularity - declarative export (exportSqlFiles) with the load(export(fb)) ≡ fb gate and an ordered layout that loads in a single pass - finalized public API (subpath exports), CLI v2 (plan/apply/prove/diff/ drift/snapshot/schema export|apply), API-REVIEW.md, drift verb Stage 10 (cutover prep): - differential harness vs the old engine (tests/differential.test.ts): buckets divergences, fails only on new-bug; ACL-normalization accepted - MIGRATION.md draft (API mapping, output-shape differences, DSL cookbook) - CI lane sharded (corpus 4-way x 3 PG versions) + benchmark job Validation: PG 17 fully green — 390/390 corpus proofs + 243 integration (execution/compaction/renames/export/policy/generative/cli/extract/loader/ fixture-validity) + 179 unit; tsc + oxlint + oxfmt clean. PG 15 corpus re-run pending (the combined parallel run starved Docker and tripped timeouts; not a logic regression). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o the rule table
Addresses the design-review findings; the proof loop is the oracle
(PG 15 and 17 both 390/390 corpus + 251 integration + 184 unit green,
differential vs old engine 37/37 zero divergences).
Architectural simplification (guardrail 3):
- per-kind graph/suppression policy moved OUT of the planner body and INTO
the rule table as KindRules flags: metadata, cascadesToChildren,
rebuildable, suppressible (FK constraints), dropRootRedirect (OWNED BY
sequences), defaclObjtype. plan.ts no longer holds METADATA_KINDS /
CASCADING_PARENTS / REBUILDABLE / isFkConstraint / DEFACL_KIND lists, so
adding a kind is self-contained in its rule entry.
Proof harness (turn declared safety metadata into verified claims, §3.7):
- relfilenode rewrite-observation: a kept table physically rewritten under
no rewriteRisk-declaring action fails the proof. This surfaced real
under-declarations — column rules now declare rewriteRisk for STORED
generated columns and volatile-default ADD COLUMN; the enum value-set
cast references its column so attribution maps to the table.
- opt-in autoSeed: synthetic rows in empty kept tables give the
data-preservation check teeth without a seed.sql.
- single-round-trip table stats (kills the per-table N+1).
Frontends / extraction:
- shadow loader: isolatedCluster vs databaseScratch modes, pg_auth_members
leak detection, mutual-FK split-your-FK diagnostic, source:sqlFiles
provenance tag.
- FactBase provenance (source) + extract({source}); snapshot capturedAt;
snapshots load tagged source:snapshot.
- security labels fully modeled: pg_seclabel/pg_shseclabel extraction +
the securityLabel global rule + rendering (unit-proven; e2e gated on a
dummy_seclabel image).
- unresolvable pg_depend endpoints recognized by the resolver but
unbuildable by the codec now emit a diagnostic (resolver/codec mismatch)
instead of silently dropping.
Policy / CLI:
- DSL v2: three target* predicates unified into one { target: {kind,schema,
name} }; the misleading global serializeParams export removed (the
planner resolves params per fact).
- CLI: --renames defaults to prompt; --accept-rename <from>=<to> closes the
interactive loop; shared parseFlags removes per-command argv boilerplate.
Hardening / docs:
- enum rename-aside temp name bumps past any existing type (collision-safe).
- differ kind-free grep guard (stage-4 gate).
- COVERAGE.md: catalog coverage, deliberate exclusions, and the two known
granularity deviations (composite-type attributes, publication
table-filters remain payload blobs — correct but coarse; deferred with
rationale rather than rushed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n members to sub-entity facts Closes the last two granularity-one deviations (COVERAGE.md): composite type attributes and publication table/schema members are now full facts at fact grain (§3.1), not payload blobs. Proven by the corpus + differential (PG 15 and 17 both 396/396 corpus + 257 integration + 189 unit; old-engine differential 39/39 zero divergences). New StableId kinds (codec + round-trip tests): - typeAttribute (schema, type, name) — a composite type's attribute - publicationRel (publication, schema, table) — a published table - publicationSchema (publication, schema) — a published schema (PG15+) Composite types: - attributes extract as typeAttribute facts parented to the type; the type payload no longer carries an attributes blob. - fresh types inline their attributes into CREATE TYPE AS (…) (delta-set); existing types are managed incrementally — ADD / DROP / RENAME ATTRIBUTE … CASCADE all work while the type is used by table columns and preserve the stored data (empirically verified). RENAME enables data-preserving composite-attribute rename detection (new capability — old engine forced a type rebuild). - ALTER ATTRIBUTE … TYPE is supported for unused composites and fails loudly with a remediation message for in-use ones (PostgreSQL forbids it there; CASCADE only reaches typed tables, not columns) — it never emits a statement that would fail at apply. Publications: - tables/schemas extract as publicationRel/publicationSchema facts; the publication payload no longer carries tables/schemas blobs. - fresh publications inline members into CREATE PUBLICATION FOR …; existing ones use ALTER PUBLICATION ADD/DROP. A per-table column-list / WHERE change replaces that member only (DROP TABLE + re-ADD) — eliminating the prior dual-SET redundancy and churning nothing else. Extraction perf: the five security-label resolver queries are now gated behind a single pg_seclabel/pg_shseclabel existence probe, so a label-free database (the common case) pays one round trip, not six. Coverage: extractor ring asserts the new facts; renames suite proves composite-attribute rename (data-preserving, in-use); 3 new corpus scenarios cover composite ADD/DROP (in-use, seeded), composite retype (unused), and publication column/WHERE change. COVERAGE.md + README updated to reflect granularity-one is now complete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ix ALTER COLUMN TYPE policy/view deps (#263) Swept the open supabase/pg-toolbelt issues and turned every diff/plan/apply behavior into a corpus scenario, verified on PG 15 and 17 (corpus 416/416 + 268 integration + 189 unit; differential vs old engine 43/43 zero divergences). Most were old-engine flaws the new architecture does not reproduce; #263 exposed a real gap, now fixed. Fix (#263 — ALTER COLUMN TYPE / DROP FUNCTION blocked by policy/view): - PostgreSQL rejects ALTER COLUMN … TYPE while a view, rule, or policy references the column (0A000). A column type change is an in-place alter, so it did not seed the forced-dependent-rebuild and the policy was only ALTER'd in place (the old engine's exact bug). rebuildsDependents is now KIND-SELECTIVE: it returns the dependent kinds to rebuild. column.type declares [view, materializedView, rule, policy] — those are dropped before the type change and recreated after; indexes/constraints are NOT force-rebuilt (PG rebuilds them itself, and dropping a PK with dependent FKs would cascade harmfully). The forced-rebuild fixpoint tracks restricted seeds vs full-destroy origins so the restriction applies only to the first hop. Verified-already-correct (old-engine bugs absent in the new architecture): - #286 domain CHECK constraint dependent of a replaced function — the generic forced-rebuild drops/recreates it (old engine skipped it). - #280 function signature change referenced by a column default / check constraint — the table is never dropped; the data-preservation proof on seeded rows confirms no recreation (old engine over-recreated the table). - #269 user object referencing an unchanged managed-schema object — the plan applies to a target that has it; no round-apply stuck statement. - #282 CREATE TYPE AS RANGE used by a table — first-class range fact with a column→type edge; the shadow loader orders the exact shuffled repro. The pg-topo classification bug does not exist in the new engine (no pg-topo in the trusted path, P1). - #219 GRANT/REVOKE on an enum-arg function — stable signature, no temp-schema artifacts. - #218 DEFERRABLE INITIALLY DEFERRED unique constraint — roundtrips via pg_get_constraintdef. New: 10 corpus scenarios + 1 shadow-loader test; COVERAGE.md gains a field-issue table. Chores #250/#244/#115 are resolved differently by the new design (benchmark harness, per-action transactionality, one graph/sort) — documented, no scenario. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…essment - pg-delta-next-linear-assessment.md: per-issue assessment of the Linear pg-delta project against the new engine (134 issues). - extension-intent.md: design for diffing stateful extensions (pgmq, pg_cron, pg_partman) as provenance-tagged intent facts in the one fact base, filtered + replayed through the same diff/graph/proof pipeline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-1555) Extension-intent Deliverable A (docs/extension-intent.md §4.3): operationally created objects (pg_partman child partitions; later pgmq queue tables) carry a `managedBy` provenance edge and are excluded from the schema fact base on both sides, so a declarative diff never DROPs them — no silent data loss. - core: EdgeKind gains "managedBy" (src/core/fact.ts). - excludeManaged (src/policy/managed.ts): fact-level subtraction of managedBy- tagged facts + descendants, mirroring subtractBaseline. Fact-level (not delta- level) keeps the proof honest: the plan you prove == the plan you run. - ExtensionHandler interface + extractWithHandlers (src/policy/extensions/): a generic, non-Supabase-coupled registry; core extract + handler captures into one fact base under the same snapshot. - pg_partman handler: reads <partman>.part_config + a recursive pg_inherits walk to tag every managed child (incl. *_default / premade). The only authoritative signal (relispartition / pg_depend can't tell a partman child from a user-declared PARTITION OF — CLI-1591). RED -> GREEN: - unit (src/policy/managed.test.ts): control proves a raw diff drops the child; with a no-op excludeManaged the exclusion asserts failed (Expected: false, Received: true) -> implemented -> 5/5 pass. A non-managed PARTITION OF removed on the desired side STILL drops (no false suppression). - integration (tests/extension-intent-partman.test.ts, supabase/postgres 17.6.1.135): against a real pg_partman parent+children, the raw diff drops the children; handler + excludeManaged stop it and preserve the parent. GREEN. Verified: 194 unit pass; check-types + format-and-lint + knip clean. Follow-up (next slice): compose handlers + excludeManaged into the CLI plan path and the provePlan re-extract; then Phase B (intent replay: create_parent / pgmq.create / cron.schedule). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
extractManaged (src/policy/extensions): core extract + handler captures, then excludeManaged — the integration extractor for both the diff sides and the proof re-extract. provePlan gains a `reextract` option (default: core extract). An integration with extension handlers passes its managed-aware extractor so the proof compares the SAME view of state it diffed; otherwise operationally-managed objects (pg_partman children) reappear as drift. RED -> GREEN: a prove roundtrip on a real pg_partman DB (seeded child row, desired adds a column to the parent) failed with 602 child-`remove` drift deltas while provePlan re-extracted via core extract; after wiring `reextract` to extractManaged it is proof-clean — verdict.ok, zero drift, zero data violations, and the seeded child row survives the migration on the clone (data-preservation). The RED also surfaced a real modeling point now captured by the test: a declarative desired that keeps pg_partman installed (no runtime create_parent) yields an ALTER-only plan that preserves the managed partitions; dropping the extension would un-manage them. Verified: 194 unit pass; check-types + format-and-lint + knip clean; 2/2 partman integration tests green (supabase/postgres 17.6.1.135). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c9c81d909
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…, seclabel crash guard - sequence and identity-column option changes now emit ONE combined statement (chained SET clauses; RESTART appended only when a bound and START both move) — separate per-field ALTERs hit invalid intermediate ranges when bounds move together (MINVALUE must be less than MAXVALUE). - orderForShadow throws ReorderParseError when pg-topo parse/discovery errors dropped statements — no caller can receive a silently-shrunk file set (analyzeForShadow keeps returning partials + diagnostics for the CLI's degrade-to-raw path). - ACL privileges/grantable aggregate with DISTINCT across grantors — the same privilege granted via two grantors no longer renders duplicate GRANTs and proof-drifts. - security labels on view/matview columns no longer crash extraction with a missing-parent throw; they surface as unresolved_security_label (blocked under --strict-coverage); the metadata fidelity gap remains tracked (#332). - @supabase/pg-delta/plan now re-exports parsePlan/serializePlan, making the documented getting-started import real. NOTE: the reviewer's 'PUBLIC dropped from mixed policy role lists' finding is a false positive — Postgres itself refuses mixed lists (WARNING: ignoring specified roles other than PUBLIC; polroles={0} is the only PUBLIC encoding, unconstructible otherwise even via allow_system_table_mods). policies.ts is correct as-is. GREEN: 776 unit, corpus 632/632 (pg17, incl. new sequence/identity alter-bounds scenarios), focused suites green, types+format clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c471de509b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…n order in creates - enum value-set rebuild temp names now check every pg_type-namespace occupant (domains, relation row types) and clip to 63 bytes with a bounded numeric probe — a table or domain named <enum>__pgdelta_replaced no longer breaks the rename-aside, and near-NAMEDATALEN enums can't truncate onto occupied names. - table creates render columns in DECLARED order: attnum is captured as the non-semantic _position (hash/diff-excluded, like composite attributes), the partitioned/inlined CREATE path sorts by it, and plain-table column folds tie-break by schema+table+attnum in the ordering phase. From-empty plans/exports of (z int, a int) now produce (z, a); order-only differences on existing tables remain undiffed by design. GREEN: 780 unit (0 snapshot churn), corpus 636/636 (pg17), export/reorder/ seed suites green, types+format clean. RED proven incl. a physical pg_attribute.attnum roundtrip assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3f9162957
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…lidation, file_fdw redaction - subscription two_phase changes no longer replace (which dropped the publisher's replication slot): PG18+ alters via DISABLE -> SET (two_phase) -> ENABLE (verified: SET (two_phase) is PG18+ grammar; PG17 rejects it), and older majors fail loudly at plan time instead of silently breaking replication. Server major travels as a non-semantic _serverMajor payload key. - a subscription created from redacted extraction with placeholder conninfo no longer gets ENABLEd against bogus credentials — the create stays disabled with an operator note; unredacted/disabled creates unchanged. - composite typeAttribute type-alters now release the old / consume the new referenced type, ordering ALTER ATTRIBUTE before DROP TYPE (mirrors the ALTER COLUMN TYPE fix). - buildFactBase rejects parent-hierarchy cycles (iterative O(n) root-reachability) — a cyclic component used to be silently invisible to rootHash/diff, fingerprinting like an empty base. - redaction allowlist keeps file_fdw's non-secret options (filename, program, null, force_not_null, force_null) — exports no longer point foreign tables at literal __OPTION_FILENAME__. GREEN: 786 unit, corpus 632/632 on PG17 AND PG18, focused suites green, types+format clean. All five findings reproduced before fixing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc0a817b37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| for (const key of path) reachesRoot.add(key); | ||
| } | ||
| for (const edge of edges) { |
There was a problem hiding this comment.
When a public buildFactBase caller or multiple extraction handlers supply the same dependency edge twice, this loop stores both copies and rollupOf() hashes both, while diff() collapses outgoing edges into an edgeKey map. Two semantically identical bases can therefore have different fingerprints but zero deltas; in particular, a plan sourced from the duplicate-bearing base can fail the apply fingerprint gate after a normal re-extraction returns one edge. Reject or deduplicate identical (from, kind, to) edges in the constructor.
Useful? React with 👍 / 👎.
| -- 'f' = foreign tables: they carry only CHECK constraints (no p/u/f/x), | ||
| -- so the contype filter already scopes them; serialized via ALTER FOREIGN | ||
| -- TABLE (constraintTarget keys off the parent's foreignTable kind). | ||
| WHERE con.contype IN ('p', 'u', 'f', 'c', 'x') AND con.conislocal |
There was a problem hiding this comment.
On PostgreSQL 18, named NOT NULL constraints are represented in pg_constraint with contype = 'n', but this filter excludes them. The column's attnotnull boolean preserves basic enforcement only; from-empty export renders a bare NOT NULL, losing an explicitly declared constraint name and any constraint comment, so subsequent migrations that address that name fail. Model these rows without independently adding a second NOT NULL clause during table creation.
Useful? React with 👍 / 👎.
| (SELECT json_agg(json_build_object( | ||
| 'name', a.attname, | ||
| 'position', a.attnum, | ||
| 'type', format_type(a.atttypid, a.atttypmod), | ||
| 'collation', CASE WHEN a.attcollation <> at.typcollation THEN ( |
There was a problem hiding this comment.
When a composite attribute has COMMENT ON COLUMN schema.type.attribute, PostgreSQL stores it on the composite's backing pg_class subobject, but this attribute JSON captures only name, position, type, and collation. No other extractor reads col_description for relkind = 'c', so comment-only drift is invisible and exports silently discard the comment; extract a metadata satellite for each commented typeAttribute and render it with the corresponding column target.
Useful? React with 👍 / 👎.
| factBase = buildFactBase( | ||
| [...factBase.facts(), ...extraFacts], | ||
| [...factBase.edges, ...extraEdges], | ||
| source, | ||
| ); |
There was a problem hiding this comment.
When core extraction produces a dangling_edge diagnostic and any extension handler contributes at least one fact or edge, this reassignment constructs a fresh FactBase without copying the original diagnostics. Because it also rebuilds from factBase.edges, where dangling edges were already pruned, those diagnostics cannot be regenerated and handler-backed profiles silently hide catalog dependency gaps that raw extraction reports. Preserve the pre-rebuild diagnostics when replacing the base.
Useful? React with 👍 / 👎.
| const r = await client.query( | ||
| `SELECT EXISTS (SELECT 1 FROM ${qualified} LIMIT 1) AS has`, | ||
| ); |
There was a problem hiding this comment.
When the loader runs as a non-superuser and a file inserts rows before enabling and forcing RLS with a policy that hides them, this SELECT EXISTS observes only the loader role's visible rows and returns false. The load then succeeds even though the declarative file contains data, and extraction discards those rows so applying the resulting schema creates an empty table instead of rejecting unsupported DML. Perform the population check through an RLS-bypassing shadow connection or otherwise fail closed when complete visibility is unavailable.
Useful? React with 👍 / 👎.
| view.get(e.from) !== undefined && | ||
| // a column that exists only in the DESIRED state is being | ||
| // created by this same plan (already with the new type) — | ||
| // there is nothing to migrate | ||
| sourceView.get(e.from) !== undefined, |
There was a problem hiding this comment.
When a table is accepted as a rename while an enum used by one of its columns removes or reorders values, the desired column has the new table name in its stable ID, so this source-side lookup returns undefined and classifies it as newly created. The rebuild consequently emits no ALTER COLUMN ... TYPE for the renamed table; its column remains bound to the renamed old enum and the final DROP TYPE fails. Translate desired column IDs through accepted table renames when deciding which existing columns must be migrated.
Useful? React with 👍 / 👎.
| CASE WHEN c.relkind = 'p' THEN pg_get_partkeydef(c.oid) END AS partition_key, | ||
| c.reloptions AS reloptions, |
There was a problem hiding this comment.
When a table sets a TOAST-specific option such as toast.autovacuum_enabled=false, PostgreSQL stores the option on the pg_class row identified by c.reltoastrelid, not in the parent table's c.reloptions. Selecting only this array makes such option-only differences invisible and from-empty exports silently restore default TOAST behavior. Read the TOAST relation's options, prefix them with toast., and include them in the table reloptions payload rendered by the existing ALTER TABLE path.
Useful? React with 👍 / 👎.
| .filter((index) => !drainedIndices.has(index)) | ||
| .sort((left, right) => | ||
| compareStatementIndices(left, right, statementNodes), | ||
| ); |
There was a problem hiding this comment.
Kahn's undrained set contains not only members of a cycle but also every acyclic statement downstream of that cycle. Sorting the entire set solely by the deterministic tie-break can therefore place a downstream statement before the cycle member it requires—for example, an early-file view depending on one member of a later-file two-view cycle. Preserve the condensation graph's ordering between strongly connected components, using the tie-break only within a cyclic component.
Useful? React with 👍 / 👎.
…uting, invalid-index repair - sequence/identity option changes append RESTART only when the old and new ranges are provably disjoint (the counter is then guaranteed invalid); overlapping-range changes preserve the live counter — the previous bound+START predicate silently reset live sequences (duplicate-key risk). A counter genuinely outside an overlapping new range fails loudly for the operator; runtime counters are not modeled state. - security labels on unmodeled pg_type kinds (e.g. table row types) now route to unresolved_security_label (blocked under --strict-coverage) instead of the orphaned_satellite info that slipped past strict mode. - pg_index.indisvalid is captured as semantic index state: an invalid index left by a failed CREATE INDEX CONCURRENTLY no longer converges against a desired valid index — it plans a drop+recreate repair. Partitioned parent indexes (relkind I, legitimately invalid mid-attach) are forced valid; their attach-state remains tracked in #332. GREEN: 788 unit, corpus 632/632 (pg17), seclabel 20 pass, index corpus 28 pass, live-counter preservation proven (last_value 500 kept), types+format clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d02a9e378
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (action.rewriteRisk) { | ||
| for (const rel of tablesReferencedBy(action)) | ||
| declaredRewriteTables.add(rel); |
There was a problem hiding this comment.
When an ALTER COLUMN TYPE targets an inheritance or partition parent, the action in plan/rules/tables.ts declares rewriteRisk but references only the parent column; PostgreSQL recursively alters and rewrites each ordinary child table. tableStats() includes those child relations, while this loop records only the parent in declaredRewriteTables, so detectViolations() reports every changed child relfilenode as an undeclared rewrite and the proof rejects an otherwise valid migration. Expand rewrite attribution through the inheritance hierarchy, including renamed descendants.
Useful? React with 👍 / 👎.
| if (wantEnabled && !suppressEnable) { | ||
| specs.push({ sql: `ALTER SUBSCRIPTION ${name} ENABLE` }); |
There was a problem hiding this comment.
When a missing or replaced subscription is desired with enabled=true under the default redaction mode, this condition deliberately omits the ENABLE action while the desired fact remains enabled. Applying the plan therefore creates a disabled subscription yet reports success, and provePlan() re-extracts enabled=false and rejects the plan as non-convergent. Either fail planning for this unreplayable state or project the desired enabled flag consistently instead of emitting a plan that cannot reach its target.
Useful? React with 👍 / 👎.
| -- DISTINCT: aclexplode() yields one row per GRANTOR, so a privilege | ||
| -- granted to one grantee by two grantors appears twice. pg-delta | ||
| -- models the EFFECTIVE privilege set, not who granted it, so grantor | ||
| -- identity is intentionally ignored — de-duplicate to avoid rendering |
There was a problem hiding this comment.
When a grantee received a privilege from another role holding grant option, collapsing all grantors into one effective ACL loses the identity needed to revoke that grant. For example, if the owner granted Alice SELECT WITH GRANT OPTION and Alice granted Bob SELECT, a desired removal of Bob emits the bare owner-side REVOKE ALL ... FROM Bob; that does not revoke Alice's grant, so direct apply can report success while Bob retains access and proof cannot converge. Retain grantor-grain ACL state or render revocations under the recorded grantor.
Useful? React with 👍 / 👎.
| } else if (dependentDestroyer === undefined && desired.has(edge.from)) { | ||
| if (producerOf.has(key)) continue; |
There was a problem hiding this comment.
When a non-relocatable extension is moved between schemas, its rule replaces it with DROP EXTENSION plus CREATE EXTENSION. References to extension-member types/functions are collapsed to dependency edges on the extension, but this branch treats the planned recreation as sufficient and ignores a surviving dependent with no destroy action—for example, a retained table column using an extension type. PostgreSQL's bare DROP EXTENSION is restrictive and fails while that dependent remains, so reject the replacement or rebuild/migrate every external dependent first.
Useful? React with 👍 / 👎.
| alter: (fact, _from, to) => { | ||
| const name = subscriptionName(fact); | ||
| const major = Number(p(fact, "_serverMajor") ?? 0); | ||
| if (major < 18) { |
There was a problem hiding this comment.
For a cross-version plan, fact is the desired-side subscription, but the generated SQL executes against the source-side database. Thus a PG17 source planned toward a PG18 snapshot reads _serverMajor=18 here and emits the PG18-only ALTER SUBSCRIPTION ... SET (two_phase) against PG17; the reverse direction incorrectly refuses an alter that the PG18 apply target supports. Read the source fact/server capability when selecting executable ALTER syntax.
Useful? React with 👍 / 👎.
…ow-ups Delegation-ready plans (V1 managed view, I1 rename identity, proof/compaction tracks) with wave/conflict matrix so parallel agents do not collide.
…t-check Second review pass against the code (action-emitter, prove, renames): - I1: rename actions are already synthesized from acceptedRenames in plan/phases/action-emitter.ts (~180-194) — brief now says keep that seam (pre-rewrite from-facts, produces=new-subtree ordering pin) instead of presenting injection as new machinery; dropped the erroneous prove.ts renamedTables entry (table/matview-only, out of I1 scope). - C1: unblocked from I1 (dependency was inherited from the old defaults-flipping design); wave 3, harness-only, prefer landing before I1 so I1's corpus gate validates both compact modes. - README: dependency graph / wave table / conflict matrix / delegation order updated to match; waves clarified as conflict groupings, not chronology; amendments log entries 8-9. - V1: changeset criterion simplified to plain patch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7fabcf72a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // within-file order: create→alter, then object→comment→…, stable by position | ||
| const statements = [...entry.items] | ||
| .sort( | ||
| (a, b) => | ||
| a.verbRank - b.verbRank || a.scopeRank - b.scopeRank || a.at - b.at, | ||
| ) |
There was a problem hiding this comment.
When a grouped export contains an object owned by a non-default role whose owner privileges were revoked, this verb-first sort moves the ACL fact's REVOKE ALL ahead of the object's ALTER ... OWNER TO. The revoke is then applied while that role is only a third party, and the later owner change remaps the current owner's default ACL to the new owner, restoring privileges that the source explicitly revoked; for example, an alice-owned table after REVOKE ALL ... FROM alice is exported in exactly this unsafe order. Preserve the planner's ordering between ownership and ACL actions rather than sorting all fact creates before alters.
Useful? React with 👍 / 👎.
| ctx.diagnostics.push(...factBase.diagnostics); | ||
| // catalog completeness: user objects in kinds we don't model are reported, | ||
| // never silently missed (review finding 1). Same snapshot, one round-trip. | ||
| ctx.diagnostics.push(...(await detectUnmodeledKinds(client))); |
There was a problem hiding this comment.
These completeness diagnostics are added only to ExtractResult.diagnostics, while serializeSnapshot() persists FactBase.diagnostics. If a non-strict snapshot is captured while an unmodeled object exists and that object is later removed, drift --strict-coverage sees neither the original warning from the snapshot nor a warning from the live database and can report no drift, even though the snapshot was known to be incomplete. Attach unmodeled_kind and the other strict-coverage diagnostics to the fact base before returning so the snapshot-side gate survives serialization.
Useful? React with 👍 / 👎.
| if (typeof obj["$bigint"] === "string" && Object.keys(obj).length === 1) { | ||
| return BigInt(obj["$bigint"]); |
There was a problem hiding this comment.
A valid public/custom-handler payload may contain an ordinary object shaped exactly like { "$bigint": "123" }; serialization leaves it unchanged, but this decoder converts it to a real bigint. The recomputed fact hash then differs, so deserializeSnapshot(serializeSnapshot(fb)) rejects the engine's own snapshot as corrupt for that payload. Use an escaped/tagged representation that cannot collide with values permitted by PayloadValue, or reject the reserved shape before serialization.
Useful? React with 👍 / 👎.
| this.#byId.set(encoded, { | ||
| fact, | ||
| encoded, | ||
| hash: contentHash(fact.payload), |
There was a problem hiding this comment.
The constructor caches contentHash(fact.payload) but stores the caller's mutable fact object by reference, and facts()/get() later expose that same object. If a public caller, handler, or retained input reference mutates a payload after construction, planning reads the new payload while hashOf, rollups, and rootHash continue representing the old value; two visibly different bases can then compare equal and skip required deltas. Deep-clone/freeze facts at the boundary and avoid returning mutable internal references.
Useful? React with 👍 / 👎.
| function seg(name: string): string { | ||
| return encodeURIComponent(name).replace(/^\.+$/, (m) => | ||
| m.replace(/\./g, "%2E"), | ||
| ); |
There was a problem hiding this comment.
This sanitizer preserves case and leaves characters such as * unescaped, so it is not a portable safe path segment. On default macOS/Windows filesystems, quoted PostgreSQL objects such as "Foo" and "foo" map to paths that refer to the same file and the later CLI write silently overwrites the first; on Windows, an identifier containing * instead makes the export fail. Use a filesystem-portable encoding and reject collisions under the target filesystem's comparison rules before writing.
Useful? React with 👍 / 👎.
…s-review Driven by a second cross-review whose headline claim was verified by live reproduction in-tree: - NEW B1: role rename + RLS policy referencing that role is a confirmed crash-class planner bug (dependency cycle between ALTER ROLE RENAME and ALTER POLICY ... TO; policies.ts roles.alter consumes/releases vs the rename action's produces/destroys; internal.ts releases edge is unconditional). Zero prior coverage. Urgent bugfix brief, before I1. - I1: payload role refs (policy.roles) in scope; discovery-diff decision (matchRenameCandidates needs diff remove/add pairs); physical-vs-canonical source model for the fingerprint/apply gate; split into I1a/I1b; fixed leftover injection/unblocks-C1 contradictions. - C1: per-mode full teardown spec (drop DBs first, dropRolesExcept, replay); genuinely harness-only (default flips are follow-up only). - P2: reframed as attributed projection audit (stage/rule attribution, acknowledged vs suspicious) — unattributed drift diffs rejected as noisy. - P3: seeder observability first (empty catch at prove.ts:257-263 swallows insert failures); per-table outcomes + coverage contract. - V1: helper internal-only; identity assertion corrected (resolveView is identity only with no extension members / managedBy provenance). - H1: per-file baseline count ratchet instead of file allowlist. - C2: evidence-gated on dual-prove divergence; cross-action folds stay pretty-only. - K1: retired — ./sql-format subpath export already exists; wording folded into D0. - README: graph/waves/matrix/delegation updated; amendments log 10-18. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… delegation Two further cross-reviews; every checkable claim verified in code before adopting: - I1: filterDeltas pinned on both diff passes (rename proposals come from policy-KEPT deltas, change-set.ts:141-162); fingerprint routing (only source.fingerprint from physicalSource); scope = role renames only; I1b owns a corpus renames opt-in (corpus plans with renames off today, engine.test.ts:50) and must delete B1's carve-out. - P2: split P2a/P2b; attribution unit is the suppressed delta/state (reference-only payload suppression, edge pruning, missing managedBy stage); stable reason codes, descendant attribution; audit computed at plan time (provePlan has no raw source FactBase). - P1: action vocabulary pinned - verbs are create|alter|drop only, no replace; budgets speak derived replacement/rename predicates over the uncompacted artifact. - P3: SQLSTATE-based skipped/failed taxonomy; keyed allowlist; strict-on-unknown. - B1: primary RED is a focused renames:"auto" test (corpus cannot express renames); carve-out restricted to role old->new pairs + over-skip negative test. - V1: guard is import/call-based per module (literal grep misses schema-export's intermediate-variable composition); "exported API" leftover fixed. - C1: two plan artifacts proved/applied as built (no downstream compact setting exists). - Changeset terminology corrected everywhere to bump types (patch|minor|none). - D0/K1: corrected pass-3 over-claim - root package transitively loads sql-format via exportSqlFiles; only focused subpaths avoid it. - Ops: engine.test.ts single-owner rule; every active brief opens with a one-line Contract; README amendments 19-28. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…residue swept - P2: owned-files rewritten for the P2a/P2b split (P2a = policy/plan-artifact plumbing, P2b = prove/CLI); per-stage classification defaults pinned, including baseline = acknowledged-but-always-visible so a baseline that captured user state stays detectable without red-lighting image upgrades. - I1: normalizer must remap FactBase.referenceOnly (encoded-id set, core/fact.ts:73); I1a changeset corrected to none (ships dark). - Sweeps: V1 no policy-barrel re-export (./policy is public); C1 stale "optional default flips" acceptance line removed; P1 summarizeActions public hatch removed (budgets read the plan artifact; prove.ts untouched); P3 taxonomy pinned to SQLSTATE class 23 (DEFAULT VALUES seeder cannot hit 428C9); C2 "already C1" leftover fixed; H1 allowlist wording -> baseline counts; conflict matrix + delegation aligned with the engine.test.ts single-owner rule. - Rejected with evidence: claim that tests/cli.test.ts doesn't exist (it does, 59.7K). README amendments 29-32. - Includes new OVERVIEW.md (ELI5/ELI10 review entry point). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…to delegate - P1: replacement predicate pinned to exact encodeId equality (name-path matching would misclassify routine overloads as replacements); stale "serialize with P2 on prove.ts" clauses removed from header/conflicts and the README matrix — P1 never touches prove.ts. - OVERVIEW: ship-order diagram orders P1 after C1 (engine.test.ts single-owner rule); C1 "Today" row corrected — the compact artifact IS proven today; the uncovered risk is compaction masking a broken uncompacted plan (--no-compact users apply a never-proven shape). Includes the mermaid/layout revisions to the tour. - README amendments 33-34; reviewer's cli.test.ts correction acknowledged. Six review rounds complete; findings decayed from a shipping crash bug (round 3) to a missing diagram edge (round 6). Next step is delegation: B1 || V1 || D0 first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e306775a29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const colFacts = view | ||
| .childrenOf(fact.id) | ||
| .filter((c) => c.id.kind === "column"); |
There was a problem hiding this comment.
When an ordinary inheritance child redeclares a compatible parent column, PostgreSQL records that merged column with both attislocal = true and a positive attinhcount, so extraction creates a column fact for it. This branch only inlines columns for partitioned parents; it therefore emits CREATE TABLE child () INHERITS (parent) followed by ALTER TABLE child ADD COLUMN ..., which fails because inheritance already created the column. Include these locally redeclared columns in the child’s initial CREATE TABLE statement.
Useful? React with 👍 / 👎.
| LEFT JOIN pg_class rc ON rc.oid = base.typrelid | ||
| AND rc.relkind IN ('r', 'p', 'f', 'v', 'm') | ||
| LEFT JOIN pg_namespace rn ON rn.oid = rc.relnamespace | ||
| WHERE base.typtype IN ('d', 'e', 'c', 'r') |
There was a problem hiding this comment.
When a PostgreSQL 14+ table column uses a multirange type, its dependency endpoint is the pg_type row with typtype = 'm', which this filter drops rather than mapping back through pg_range.rngmultitypid to the modeled range fact. The planner consequently sees no column-to-type edge; its weight ordering emits or folds the column into CREATE TABLE before CREATE TYPE ... AS RANGE, so from-empty plans and schema exports fail with “type does not exist.” Resolve multirange OIDs to the range fact that creates them.
Useful? React with 👍 / 👎.
| throw error; | ||
| } | ||
|
|
||
| const planOptions: PlanOptions = { |
There was a problem hiding this comment.
When a nonempty directory contains only statements that leave no managed catalog facts—such as SELECT 1, SET statement_timeout, or CREATE TEMP TABLE—the loader accepts every file and this call returns an effectively empty database-scoped desired state. planSchemaFiles then plans that state against the target and schema apply drops every managed target object without requiring confirmation, bypassing the existing empty/comment-only input safeguard. Refuse a load whose resolved managed view contains no declared state.
Useful? React with 👍 / 👎.
| for (const file of pending) { | ||
| try { | ||
| await applyFile(client, file.sql); |
There was a problem hiding this comment.
When raw files use session-scoped directives across file boundaries, disabling the pg-topo reorder is insufficient because this retry loop still removes successful files and runs failed files again after later SET/RESET files have changed the session. For example, SET ROLE alice, a view that initially fails on a missing table, RESET ROLE, and the table creation cause the retried view to be created as the original user rather than Alice, silently changing ownership in the extracted desired state. Treat session-setting spans as retry barriers or reject cross-file session state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdbb7b4cbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rangesDisjoint( | ||
| str(source.payload["minValue"]), | ||
| str(source.payload["maxValue"]), | ||
| str(p(fact, "minValue")), | ||
| str(p(fact, "maxValue")), | ||
| ) |
There was a problem hiding this comment.
When a live positive-increment sequence is at 900 and its maximum narrows from 1000 to 800, the old and new ranges still overlap, so this predicate skips RESTART; PostgreSQL retains the current counter, causing the next nextval() to fail at the new maximum (or cycle unexpectedly), while the proof sees matching catalog options and reports convergence. Fresh evidence after the earlier fix is that the new disjoint-only check still does not establish that the live counter lies inside the overlap; probe the counter or fail loudly whenever a range narrows without proving it remains valid. The identity-sequence helper uses the same predicate and is affected too.
Useful? React with 👍 / 👎.
| WHEN e.classid = 'pg_trigger'::regclass THEN trg.id | ||
| WHEN e.classid = 'pg_namespace'::regclass THEN nsp.id | ||
| ELSE NULL |
There was a problem hiding this comment.
When an installed extension declares another extension in its control file's requires list, PostgreSQL records a normal pg_extension → pg_extension dependency, but this resolver has no pg_extension endpoint and sends both sides through ELSE NULL. The fact graph therefore loses the required ordering: from-empty replay fails whenever the dependent extension sorts before its prerequisite, while teardown fails in the opposite name ordering because PostgreSQL refuses to drop a prerequisite before its dependent. Resolve direct extension endpoints to their extension facts so create and drop actions follow the catalog dependency.
Useful? React with 👍 / 👎.
| } else { | ||
| parts.push( | ||
| `LC_COLLATE = ${lit(str(p(fact, "lcCollate")))}`, | ||
| `LC_CTYPE = ${lit(str(p(fact, "lcCtype")))}`, |
There was a problem hiding this comment.
A user collation created with CREATE COLLATION app.c FROM pg_catalog."default" is extracted with collprovider = 'd'; its libc locale fields are null because the provider delegates to the database default. This catch-all branch treats it as libc and calls str() on those null fields, so planning or exporting the otherwise valid collation throws rule rendering: expected a scalar, got null. Add a provider-d rendering path, such as recreating it from the default collation, instead of routing it through the libc fields.
Useful? React with 👍 / 👎.
| for (const row of await q(` | ||
| SELECT n.nspname AS schema, t.typname AS name, r.rolname AS owner, | ||
| format_type(t.typbasetype, t.typtypmod) AS base_type, | ||
| t.typnotnull AS not_null, t.typdefault AS default_expr, |
There was a problem hiding this comment.
When a domain default was created through a search path, such as SET search_path = app; CREATE DOMAIN app.d AS integer DEFAULT f(), typdefault can retain the unqualified text f(). Extraction's canonical search path cannot change that stored string, and every generated plan explicitly applies with search_path = pg_catalog, so replay fails even though the dependency graph correctly creates app.f first. Read pg_get_expr(t.typdefaultbin, 0) here, as the column-default extractor already does, so non-catalog references are emitted schema-qualified.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf80086f70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const id = fact.id as { schema: string; name: string }; | ||
| return [ | ||
| { | ||
| sql: `CREATE MATERIALIZED VIEW ${rel(id.schema, id.name)}${reloptionsWithClause(fact)} AS ${str(p(fact, "def"))}`, |
There was a problem hiding this comment.
When the desired materialized view was created WITH NO DATA, this unconditional CREATE MATERIALIZED VIEW ... AS ... uses PostgreSQL's WITH DATA default, executing the view query and populating it during apply/export. extractViews also omits pg_class.relispopulated, so the resulting state change is invisible to the fingerprint and proof can report convergence; capture the populated flag and append WITH NO DATA when it is false.
Useful? React with 👍 / 👎.
| // ── roles (cluster-level) ──────────────────────────────────────────── | ||
| for (const row of await q(` | ||
| SELECT r.rolname AS name, r.rolsuper, r.rolinherit, r.rolcreaterole, | ||
| r.rolcreatedb, r.rolcanlogin, r.rolreplication, r.rolbypassrls, |
There was a problem hiding this comment.
For roles configured with CONNECTION LIMIT—especially CONNECTION LIMIT 0—this extraction query omits pg_roles.rolconnlimit, so limit-only drift compares equal and a from-empty cluster plan recreates the role with PostgreSQL's unlimited default. This removes an explicit connection/resource restriction without producing a delta or proof failure; include the limit in the role payload and render it in both CREATE and ALTER actions.
Useful? React with 👍 / 👎.
| // ── role memberships (cluster-level; multi-grantor rows deduped) ───── | ||
| for (const row of await q(` | ||
| SELECT r1.rolname AS role, r2.rolname AS member, | ||
| bool_or(m.admin_option) AS admin |
There was a problem hiding this comment.
On PostgreSQL 16+, a membership granted with INHERIT FALSE or SET FALSE is collapsed here to only its admin_option; it therefore compares equal to a default membership, and from-empty replay emits a bare GRANT that restores the more permissive defaults. This can let the member inherit privileges or use SET ROLE despite the source explicitly forbidding it, while proof accepts the broadened membership because those fields are never extracted; version-gate and preserve inherit_option and set_option in the membership fact and SQL.
Useful? React with 👍 / 👎.
| return [ | ||
| { | ||
| sql: | ||
| `CREATE SEQUENCE ${rel(id.schema, id.name)} AS ${str(p(fact, "dataType"))}` + |
There was a problem hiding this comment.
When the desired sequence is UNLOGGED, this renderer always emits CREATE SEQUENCE, producing a logged sequence instead. Because extractSequences also omits pg_class.relpersistence, a persistence-only difference is invisible and proof accepts the changed crash-recovery and durability semantics; capture persistence and render CREATE UNLOGGED SEQUENCE, routing logged/unlogged transitions through replacement as needed.
Useful? React with 👍 / 👎.
| lcCollate: (raw["collcollate"] as string | null) ?? null, | ||
| lcCtype: (raw["collctype"] as string | null) ?? null, |
There was a problem hiding this comment.
For a PostgreSQL version that exposes pg_collation.collicurules, an ICU collation created with a RULES clause is reduced here to provider, locale, and determinism. A rules-only drift therefore compares equal, and from-empty replay recreates the collation without its tailoring while proof accepts it; this can change ordering and equality behavior used by dependent indexes and constraints, so capture the version-gated rules field and include it in replacement/create rendering.
Useful? React with 👍 / 👎.
| COALESCE((SELECT array_agg(cfg ORDER BY cfg) | ||
| FROM pg_db_role_setting s, unnest(s.setconfig) cfg | ||
| WHERE s.setrole = r.oid AND s.setdatabase = 0), | ||
| '{}')::text[] AS config |
There was a problem hiding this comment.
In cluster scope, a role with COMMENT ON ROLE produces exactly the same fact as an uncommented role because this query never reads the shared-object description and the role is pushed without metadata. Comment-only drift is consequently invisible and cluster exports silently discard the comment even though the generic renderer supports role comment targets; read shobj_description(r.oid, 'pg_authid') and emit the normal comment satellite.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a537d02af8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| JOIN pg_namespace n ON n.oid = c.relnamespace | ||
| JOIN pg_type t ON t.oid = a.atttypid | ||
| LEFT JOIN pg_attrdef ad ON ad.adrelid = c.oid AND ad.adnum = a.attnum | ||
| WHERE c.relkind IN ('r', 'p', 'f') AND a.attnum > 0 AND NOT a.attisdropped |
There was a problem hiding this comment.
When a view or materialized view column has COMMENT ON COLUMN or a column-level grant such as GRANT SELECT (col), this filter excludes its pg_attribute row, while extractViews() reads only relation-level comments and ACLs. Those metadata-only differences therefore compare equal, and a from-empty schema export omits the column comment or privilege entirely; extract the applicable view-column metadata without treating those columns as independently addable table columns.
Useful? React with 👍 / 👎.
| pg_get_viewdef(c.oid) AS def, | ||
| c.reloptions AS reloptions, |
There was a problem hiding this comment.
When a materialized view is declared with USING <access_method> or a nondefault TABLESPACE, PostgreSQL records those choices in pg_class.relam and reltablespace, but this extraction captures only its definition and reloptions. Such drift is invisible, and from-empty replay recreates the materialized view with the target database's default access method and tablespace while proof still reports convergence; capture and render both storage attributes.
Useful? React with 👍 / 👎.
| { | ||
| kind: "cast", | ||
| classid: "pg_cast", | ||
| oid: "c.oid", | ||
| name: "format_type(c.castsource, NULL) || ' AS ' || format_type(c.casttarget, NULL)", | ||
| from: "pg_cast c", | ||
| }, |
There was a problem hiding this comment.
When a schema contains a user-created CREATE CONVERSION, no extractor models its pg_conversion row and this completeness probe list does not inspect that catalog. Consequently even --strict-coverage reports the catalog as fully covered, while snapshots and from-empty exports silently omit the conversion; add a provenance-aware pg_conversion probe so strict mode fails instead of providing a false completeness guarantee.
Useful? React with 👍 / 👎.
| for (const row of await q(` | ||
| SELECT p.pubname AS name, r.rolname AS owner, | ||
| p.puballtables AS all_tables, p.pubviaroot AS via_root, | ||
| p.pubinsert, p.pubupdate, p.pubdelete, p.pubtruncate, |
There was a problem hiding this comment.
On PostgreSQL 18, a publication configured with WITH (publish_generated_columns = stored) differs from the default by whether stored generated columns are replicated, but this query captures only the older publish flags. A setting-only drift therefore compares equal, and from-empty replay recreates the publication with the default none, silently stopping those columns from being published while proof accepts the result; version-gate and preserve the generated-column publication option.
Useful? React with 👍 / 👎.
| CASE WHEN c.relkind = 'p' THEN pg_get_partkeydef(c.oid) END AS partition_key, | ||
| c.reloptions AS reloptions, |
There was a problem hiding this comment.
When a table was declared with CREATE TABLE ... OF composite_type, PostgreSQL records the relationship in pg_class.reloftype, but the table payload omits it and the renderer emits an ordinary CREATE TABLE. A typed table reconstructed from empty therefore loses the linkage that makes later ALTER TYPE ... CASCADE operations propagate to it; dependency edges alone only affect ordering and cannot recreate this relationship, so capture the referenced type and render the OF form.
Useful? React with 👍 / 👎.
| -- composite's attribute signature in so such a change | ||
| -- flips content to count-only — an additive ALTER TYPE … | ||
| -- ADD ATTRIBUTE is lossless, not a data mutation (one | ||
| -- level deep; nested composites are a known gap). | ||
| SELECT string_agg( | ||
| ca.attname || ':' || ca.atttypid::text, ',' |
There was a problem hiding this comment.
When a populated table column uses composite outer, and one of outer's attributes uses composite inner, adding or changing an attribute of inner legitimately changes the row's textual representation. This signature records only outer's immediate attribute OIDs, so it remains unchanged and the resulting content-hash difference is reported as data corruption, causing provePlan() to reject a valid nested-composite migration; recursively incorporate nested composite structure or otherwise downgrade this case to count-only coverage.
Useful? React with 👍 / 👎.
… seed-coverage contract (#350) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a987a74ab2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ${twoPhaseExpr} AS two_phase, | ||
| ${runAsOwnerExpr} AS run_as_owner, | ||
| ${originExpr} AS origin, | ||
| obj_description(s.oid, 'pg_subscription') AS comment |
There was a problem hiding this comment.
When a subscription has a COMMENT ON SUBSCRIPTION, this expression always returns null because pg_subscription is a shared catalog and its comments are stored in pg_shdescription, not pg_description. The comment therefore produces no satellite fact, so comment-only drift is invisible and a from-empty export drops the comment while proof still reports convergence; use shobj_description for subscriptions.
Useful? React with 👍 / 👎.
| SELECT sl.provider, sl.label, s.subname AS name | ||
| FROM pg_seclabel sl | ||
| JOIN pg_subscription s ON s.oid = sl.objoid AND sl.classoid = 'pg_subscription'::regclass |
There was a problem hiding this comment.
When a security-label provider labels a subscription, the row is stored in pg_shseclabel because pg_subscription is shared across the cluster. This pg_seclabel join never resolves that label; the later shared-label scan instead reports it as unsupported, so normal exports omit modeled subscription metadata and strict coverage rejects it. Read pg_shseclabel and restrict the joined subscription to the current database.
Useful? React with 👍 / 👎.
| AND de.objid = ${oid} AND de.deptype = 'e')`; | ||
| } | ||
|
|
||
| const PROBES: readonly UnmodeledProbe[] = [ |
There was a problem hiding this comment.
On PostgreSQL 15+, grants such as GRANT SET ON PARAMETER log_statement TO app create user-managed rows in pg_parameter_acl, but this completeness list neither models nor probes that catalog. Consequently privilege-only drift is invisible even under --strict-coverage; in particular, a desired revoke can leave a target role's SET or ALTER SYSTEM privilege intact while drift and proof report success. Add a pg_parameter_acl unmodeled-kind probe until parameter privileges are modeled.
Useful? React with 👍 / 👎.
| ${disableOnErrorExpr} AS disable_on_error, | ||
| ${twoPhaseExpr} AS two_phase, | ||
| ${runAsOwnerExpr} AS run_as_owner, | ||
| ${originExpr} AS origin, |
There was a problem hiding this comment.
On PostgreSQL 16+, pg_subscription.subpasswordrequired records the password_required option, but the version-gated option list and payload omit it. A difference in this security setting therefore compares equal; notably, planning from a target with password_required = false toward a desired true leaves the weaker setting in place, while a from-empty replay always uses the server default and proof accepts it. Capture the field and render it in CREATE and ALTER option handling.
Useful? React with 👍 / 👎.
| (SELECT ic.relname FROM pg_index i | ||
| JOIN pg_class ic ON ic.oid = i.indexrelid | ||
| WHERE i.indrelid = c.oid AND i.indisreplident) AS replica_identity_index, |
There was a problem hiding this comment.
When a table has been configured with ALTER TABLE ... CLUSTER ON index, PostgreSQL stores that persistent choice in pg_index.indisclustered, but extraction records only the replica-identity index. Cluster-designation-only drift therefore compares equal, and a from-empty export recreates the indexes without the designation, so a later bare CLUSTER table no longer uses the intended index while proof still reports convergence. Capture the clustered index name and render CLUSTER ON/SET WITHOUT CLUSTER transitions.
Useful? React with 👍 / 👎.
| for (const r of thePlan.acceptedRenames ?? []) { | ||
| if ( | ||
| (r.from.kind === "table" || r.from.kind === "materializedView") && | ||
| (r.to.kind === "table" || r.to.kind === "materializedView") | ||
| ) { |
There was a problem hiding this comment.
When an accepted rename changes a populated schema's name, the rename action destroys and produces the entire schema subtree, so every contained table is added to recreatedTables. This recovery loop only maps table-root renames and leaves those schema descendants marked as recreated; the final proof therefore skips all their row, content, and rewrite checks even though the tables were merely moved. Expand schema renames into old-to-new relation mappings and remove the carried tables from recreatedTables.
Useful? React with 👍 / 👎.
| } finally { | ||
| // ALWAYS restore session state before the client returns to the pool, | ||
| // on success or failure — RESET ALL must not be skipped by the catch's | ||
| // early return, and a reset failure must not flip the action's outcome. | ||
| await client.query("RESET ALL").catch(() => {}); |
There was a problem hiding this comment.
When the supplied pool initializes connections with caller-owned session settings or custom GUCs, executing any nontransactional plan segment runs RESET ALL and permanently clears those settings before the client is returned to the pool. Later segments on the same client—and unrelated future borrowers—then run under different session configuration. Track and reset only lock_timeout, statement_timeout, and the plan preamble settings, as the rendered-file path already does, rather than resetting the entire session.
Useful? React with 👍 / 👎.
…eed files (#354) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dbf75c470
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const rel of tablesReferencedBy(action)) | ||
| declaredRewriteTables.add(rel); |
There was a problem hiding this comment.
When a rewrite-risk action targets a partitioned or inherited parent—for example, ALTER COLUMN TYPE on the parent—PostgreSQL can rewrite each physical child table. tableStats() records those relkind = 'r' children, but this loop adds only relations directly referenced by the action, so the changed child relfilenodes are later reported as undeclared rewrite violations and provePlan() rejects a valid migration. Expand rewrite declarations through pg_inherits descendants before comparing the table statistics.
Useful? React with 👍 / 👎.
| (r, i) => | ||
| `(SELECT count(*) FROM ${qte(r.schema)}.${qte(r.name)}) AS c${i}`, | ||
| ) |
There was a problem hiding this comment.
For a database with more than PostgreSQL's target-list limit of 1,664 user tables, this builds one SELECT expression per table and the proof fails before applying anything with “target lists can have at most 1664 entries.” The content-fingerprint query below has the same shape for nonempty tables. Execute these statistics in bounded batches so provePlan() remains usable on large schemas.
Useful? React with 👍 / 👎.
| const tables = await client.query(` | ||
| SELECT n.nspname AS schema, c.relname AS name | ||
| FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace | ||
| WHERE c.relkind = 'r' | ||
| AND ${USER_SCHEMA_FILTER} | ||
| AND ${notExtensionMember("pg_class", "c.oid")}`); |
There was a problem hiding this comment.
When a declarative file performs INSERT, UPDATE, or DELETE through a foreign table, applyFile() executes and commits that operation against the configured remote server, but this post-load check inspects only ordinary local tables (relkind = 'r'). The loader can therefore accept the file without any data_statement diagnostic while the supposedly disposable shadow load has mutated an external, potentially production database. Foreign-table DML must be rejected before execution rather than relying on local row observation.
Useful? React with 👍 / 👎.
| try { | ||
| await client.query(row.def); |
There was a problem hiding this comment.
When the loaded schema contains an enabled DDL event trigger, replaying each pg_get_functiondef() here runs CREATE OR REPLACE FUNCTION in its own committed transaction and fires the trigger again for every routine. An audit trigger can consequently insert rows and make an otherwise valid load fail the DML check, while triggers that alter catalog state can silently contaminate the extracted desired state with effects not declared by any input file. Validate each definition inside a transaction that is always rolled back so only the validation error is observed.
Useful? React with 👍 / 👎.
| ARRAY(SELECT format_type(t.t, NULL) | ||
| FROM unnest(p.proargtypes) WITH ORDINALITY AS t(t, ord) | ||
| ORDER BY t.ord)::text[] AS identity_args, | ||
| a.aggkind AS agg_kind, a.aggnumdirectargs AS num_direct_args, |
There was a problem hiding this comment.
When a user aggregate is declared with a variadic final argument, pg_proc.provariadic distinguishes it from an ordinary array argument, but this extraction records only the flattened proargtypes. The renderer consequently recreates agg(VARIADIC integer[]) as agg(integer[]), so expanded calls such as agg(1, 2) stop resolving while re-extraction and proof still compare equal. Capture the variadic marker and render VARIADIC on the final aggregate argument.
Useful? React with 👍 / 👎.
| to: parseId(e.to), | ||
| kind: e.kind, | ||
| })); | ||
| const factBase = buildFactBase(facts, edges, "snapshot"); |
There was a problem hiding this comment.
When a library caller snapshots a managed view produced by policy or scope projection, some facts can be present only as dependency anchors in FactBase.referenceOnly. Deserialization rebuilds the base without that set, and the digest check does not notice because rootHash excludes reference-only metadata; the restored snapshot can therefore emit add/remove/set actions for platform facts that the original view deliberately made non-manageable. Serialize and restore the encoded reference-only IDs as part of the snapshot format.
Useful? React with 👍 / 👎.
3 participants
What this is
A clean-room rebuild of the
pg-deltaschema-diff engine and the hard switchthat makes it the published
@supabase/pg-delta. It compares two PostgreSQLschemas, emits an ordered DDL migration, and — new in this engine — proves
that migration converges with your data intact before you trust it.
The legacy per-object-type engine is deleted; the clean-room engine (developed
under the working name
pg-delta-next) is promoted intopackages/pg-deltaandpublished as a breaking-change alpha under the familiar name and
pgdeltabinary. Nothing carries over — the CLI, the public API, and every persisted
artifact format are new. The old engine remains on npm as
1.0.0-alpha.31andin git history.
The bet
The engine never parses SQL to understand it. Every state is resolved by a real
PostgreSQL instance (a live DB, or a shadow DB populated from your
.sqlfiles)and read back out of the catalog. PostgreSQL knowledge collapses from eight
forms to two: the extraction queries, and one rule table.
Everything flows at one grain — the fact (a content-addressed table, column,
constraint, policy, grant, …). So:
(state proof) and check seeded rows survive (data proof).
By the numbers (old engine measured at cutover; now removed)
pg-deltapg-deltaLOC is current (
find src -name '*.ts' ! -name '*.test.ts'); the engine grewwith kind coverage / export / baselines since the −79% snapshot. The extract-latency
row is a cutover benchmark — the legacy engine has been removed, so that ratio is
a historical snapshot, not something this PR can re-run. The other rows are
architectural and still exact.
The hard switch
packages/pg-delta-next→
packages/pg-delta; renamed the package to@supabase/pg-delta(un-private),kept the
pgdeltabinary, restored publish metadata, continued the1.0.0-alpha.xlineage.bun run build(tscwithrewriteRelativeImportExtensions) emitsdist/*.js+*.d.tsfrom the.ts-extension source; dualbun(TS source) /import/require/types/
default(compiled dist) exports across all 11 subpaths. Verified end-to-endby the Deno golden-path e2e (
bun run build+npm pack+ Deno import).15-shard file sharding, alpine-tags hash) is replaced by the engine's
testcontainers jobs folded into
tests.yml—pg-delta-unit,pg-delta-corpus(PG 14–18 × 4 shards),pg-delta-integration(PG 14–18) —with the
pg-delta: Integration (PostgreSQL 15/17)andpg-delta: Unit testsstatus-check names preserved for branch protection. Local coverage
instrumentation is retained (opt-in
BUN_COVERAGE)..github/agents/pg-toolbelt.md(CLAUDE.md / AGENTS.mdsymlinks) is rewritten for the new engine; README, getting-started, overview,
and CLI usage strings now say
@supabase/pg-delta/pgdelta.Notable capabilities (on the branch)
schema exportis a human-facing artifact:pretty-printed by default (
--no-formatopts out), validated constraints foldinline into
CREATE TABLE, indexes co-locate with their owning relation, andmutually-referencing foreign keys round-trip — all under the
load(export(db)) ≡ dbfidelity gate.--profilecan declare abaseline(a
snapshotsubtracted from both sides), so platform objects (base-imageroles, extension-owned schemas) stay invisible with no per-command flag. The
baseline's digest is stamped on plan/export artifacts and reconciled at
apply/prove —
plan == prove == apply, and a swapped/edited/missing baselinefails loud.
diff/drift/snapshotgained--profilefor parity.raw|supabase| custom) with extension intenthandlers (
pg_cron,pg_partman), assumed schemas/roles, and secret redaction(FDW options, subscription conninfo) carried through apply/prove.
Validation
PG 14–18 — each plan applied to a clone, re-extracted, and checked for state
convergence + data preservation.
check-types/knip/ unit suite,full export family + the new extension-member export regression, and the full
PG17 corpus (588) all green.
Changesets
Collapsed to two entries so the release changelog is readable: a single
@supabase/pg-deltamajor "clean-room rewrite" entry (the switch + allengine work fold into one breaking alpha), plus the genuine
@supabase/pg-topominor bump. The engine-hardening backlog (open Codex findings for the extract /
plan completeness gaps, none switch regressions) is tracked in
docs/roadmap/pg-delta-next-follow-ups.md.
Notes for reviewers
into three audience paths (overview /
getting-started /
architecture); build/review history is in
build-log.md.
historical narrative; user-facing surfaces and the scoped package name are
@supabase/pg-delta.🤖 Generated with Claude Code