From 800f12458e8b8e1551008e9c1f01a7590d365335 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Thu, 20 Aug 2026 15:56:57 +1000 Subject: [PATCH 1/3] docs: explain the four-step sequence and disambiguate needs-rewrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The committed-prefix section showed the four-step SET NOT NULL sequence without saying what the original statement was or why it decomposes that way — a per-step table now covers purpose, lock profile, and budget class. "Needs-rewrite" in both design docs read as if the submitted SQL needed rewording; it means a PostgreSQL table rewrite (the copy-and-swap executor's job) — safer-sequence substitution stays on the native-safe path. --- docs/execution-model.md | 24 +++++++++++++++++++++++- docs/high-level-design.md | 6 +++++- docs/low-level-design.md | 7 +++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/docs/execution-model.md b/docs/execution-model.md index 2c889ee..9f0fc62 100644 --- a/docs/execution-model.md +++ b/docs/execution-model.md @@ -70,7 +70,29 @@ shape: steps 1 through N−1 each committed, step N rolled back, steps N+1 onward never attempted. The leading run of completed steps is the **committed prefix** — no holes, no in-limbo steps. -The four-step `SET NOT NULL` sequence, failing at step 3: +The running example is a single submitted statement: + +```sql +ALTER TABLE users ALTER COLUMN email SET NOT NULL; +``` + +Run as-is, PostgreSQL takes `ACCESS EXCLUSIVE` and scans every row to prove no +NULLs exist — blocking all reads and writes for the whole scan. The planner +substitutes a four-step native sequence that moves the scan off the exclusive +lock, exploiting the fact that PostgreSQL (12+) skips the `SET NOT NULL` scan +when a validated `CHECK` constraint already proves the invariant: + +| # | Statement | What it does | Lock (duration) | Budget class | +|---|---|---|---|---| +| 1 | `ADD CONSTRAINT … CHECK (email IS NOT NULL) NOT VALID` | Installs the scaffold: enforced for new writes immediately; existing rows not yet checked | `ACCESS EXCLUSIVE` (brief — `NOT VALID` skips the scan) | brief | +| 2 | `VALIDATE CONSTRAINT …` | Scans the table to prove existing rows satisfy the invariant — the long part | `SHARE UPDATE EXCLUSIVE` (long, but reads and writes continue) | validate (own overall bound) | +| 3 | `ALTER COLUMN email SET NOT NULL` | The actual change — now a pure catalog flip, because the validated `CHECK` proves the invariant so no scan is needed | `ACCESS EXCLUSIVE` (brief) | brief | +| 4 | `DROP CONSTRAINT …` (the scaffold) | Removes the now-redundant scaffold constraint | `ACCESS EXCLUSIVE` (brief) | brief | + +The exclusive locks are held only for instant catalog flips; the single +full-table scan runs under a lock that blocks neither reads nor writes. + +The same sequence, failing at step 3: ```diagram step 1 ADD CONSTRAINT ... CHECK (col IS NOT NULL) NOT VALID ── committed ─┐ committed diff --git a/docs/high-level-design.md b/docs/high-level-design.md index 5032fe0..8ddd632 100644 --- a/docs/high-level-design.md +++ b/docs/high-level-design.md @@ -102,7 +102,11 @@ the planner's classifier: almost no parsing logic. This path lives at the `migrate` front door. - **Classified planning path (Phases 2.1–2.4).** Parse the statement and introspect the live schema to **predict the path up front** — native-safe, needs-rewrite, or refuse — without trial - execution. The planner drives `diff` and `migrate --dry-run`, and its classified route drives + execution. *Needs-rewrite* refers to a **table rewrite**: PostgreSQL would rebuild the + whole table (a full-table copy under `ACCESS EXCLUSIVE`), so executing the change online + is the future copy-and-swap executor's job — it does not mean the submitted SQL merely + needs rewording (that is the planner's safer-sequence substitution, which stays on the + native-safe path). The planner drives `diff` and `migrate --dry-run`, and its classified route drives `migrate`'s execution: a blocking submitted form is substituted with the planner's safer native sequence instead of incurring a wasted/aborted blind attempt. diff --git a/docs/low-level-design.md b/docs/low-level-design.md index 7a9129f..5454d1b 100644 --- a/docs/low-level-design.md +++ b/docs/low-level-design.md @@ -52,8 +52,11 @@ strategies rather than the whole product: - **Planner** — parse the change (imperative `--alter` or declarative desired-state diff), introspect the live schema, and **classify** every operation as *native-safe*, - *needs-rewrite*, or *refuse*. The planner decides **what** must change. It has no idea - *how* any executor works. + *needs-rewrite*, or *refuse*. *Needs-rewrite* means PostgreSQL would rewrite the + **table** — a full-table copy under `ACCESS EXCLUSIVE` — so the change belongs to the + copy-and-swap executor (not that the SQL text needs rewording; safer-sequence + substitution is a native-safe outcome). The planner decides **what** must change. It has + no idea *how* any executor works. - **Router** — given the classified plan plus policy and cluster facts (reversibility required? app schema-version aware? logical replication available? table shape?), **choose the executor** for each change. The router decides **which strategy**, and is the single From d1bec778741c25707fab1f498ca5c0910e23535f Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Thu, 20 Aug 2026 15:56:57 +1000 Subject: [PATCH 2/3] docs: add safer-sequences page (the improve path) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The improve path had no human-first explanation of why the substituted sequence is safer — the ADD CONSTRAINT UNIQUE two-step is worked through as the example (same end state, different locking, failure modes, transactionality, cost), plus what the engine adds over running the idiom by hand, the substitutions made today, and the typed caveats. Linked from the README's Improve paragraph and the docs index. --- README.md | 2 + docs/README.md | 1 + docs/safer-sequences.md | 118 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 docs/safer-sequences.md diff --git a/README.md b/README.md index 0af0a34..ac7e906 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,8 @@ machine-readable shape is in `migrate --dry-run` shows exactly what would run, as compiler-style diagnostics with a doc anchor per finding (exit 0 — the plan is executable). The demo above records the whole flow — dry run, real run, catalog proof; +why the substituted sequence is safer — same end state, different locking +— is worked through in [docs/safer-sequences.md](docs/safer-sequences.md); the machine-readable shape is in [docs/cli-output-examples.md](docs/cli-output-examples.md). diff --git a/docs/README.md b/docs/README.md index 72eeea4..9e068d7 100644 --- a/docs/README.md +++ b/docs/README.md @@ -34,6 +34,7 @@ Aurora-only. Why that combination is the product is [vision.md](vision.md); star | [tcb-model.md](tcb-model.md) | The **TCB model** — the trusted-computing-base partition of the engine: which components are the small trusted core that enforces the invariant registry vs the untrusted periphery, the never-trust-callers rule, domain types that make illegal states unrepresentable, the in-TCB engineering rules (from TigerBeetle TIGER_STYLE, s2n-tls, qmail, bitcoin-core), the verification ladder, and the per-side AI-assisted development policy. | | [plan-report.md](plan-report.md) | The **plan report contract** — the versioned JSON shape both front doors emit for dry-run plans: fields, closed vocabularies, the fingerprint identity, required consumer behavior for unknown versions/values, and one generated example per source (pinned by test). | | [cli-output-examples.md](cli-output-examples.md) | **CLI output examples** — one real, captured JSON output per shape the CLI produces: the plan report for every dry-run disposition (execute, safer-sequence substitution, rewrite-required, backend-unavailable, refusal, destructive), the execution verdict, exit codes, the linter, and diff. | +| [safer-sequences.md](safer-sequences.md) | **Safer-sequence substitution** (the improve path) — how the planner replaces a native-but-blocking form with the ordered online sequence that reaches the same end state: a worked `ADD CONSTRAINT … UNIQUE` comparison (locking, failure modes, transactionality, cost), what the engine adds over running the idiom by hand, the substitutions made today, and the typed caveats. | | [execution-model.md](execution-model.md) | The **execution model** — why safer sequences run autocommit-each-step with no wrapping transaction (PostgreSQL forbids it for the online forms), the **committed prefix** a mid-sequence failure leaves, how the verdict reports the boundary, and the per-sequence partial-failure contracts with their retry paths. Read this to answer "if a multi-step change fails halfway, what state is my table in?" | | [limitations.md](limitations.md) | The **current limitations** — schema changes pg-sprite refuses today, why they are unsafe or unsupported, and where an operator must act outside the engine. | | [lint-report.md](lint-report.md) | The **lint report contract** — the versioned JSON shape `pg-sprite lint` emits for offline CI gating: finding fields (verbatim SQL, line/column), the codes table, severities and exit behavior, the offline-conservatism rules, and how the contract versions relative to the plan report. | diff --git a/docs/safer-sequences.md b/docs/safer-sequences.md new file mode 100644 index 0000000..4e6df3e --- /dev/null +++ b/docs/safer-sequences.md @@ -0,0 +1,118 @@ +# Safer-sequence substitution (the improve path) + +The planner recognizes a submitted statement that is **native but blocking** — +PostgreSQL can do the work without a table rewrite, but the form as written +holds `ACCESS EXCLUSIVE` while it does real work — and substitutes the +**safer native sequence**: an ordered set of statements that reaches the same +declared end state while confining every exclusive lock to a brief, +metadata-only catalog flip. The classification is `safer-idiom` in the +[plan report](plan-report.md); `migrate` executes the substitution, +`migrate --dry-run` and [`suggest`](suggest-report.md) show it, and +[`lint`](lint-report.md) flags the blocking form offline. Watch the whole +flow in [demos/improve.gif](demos/improve.gif). + +## Table of contents + +- [A worked example: `ADD CONSTRAINT … UNIQUE`](#a-worked-example-add-constraint--unique) +- [Same end state, different path](#same-end-state-different-path) +- [What the engine adds over running the sequence by hand](#what-the-engine-adds-over-running-the-sequence-by-hand) +- [The substitutions the planner makes today](#the-substitutions-the-planner-makes-today) +- [Caveats are typed, not prose](#caveats-are-typed-not-prose) + +## A worked example: `ADD CONSTRAINT … UNIQUE` + +The submitted change: + +```sql +ALTER TABLE users ADD CONSTRAINT users_email_key UNIQUE (email); +``` + +Run as written, PostgreSQL builds the backing unique index **under +`ACCESS EXCLUSIVE`** — every read and write on `users` queues behind the +lock for the full table scan and sort. On a large or hot table that is an +outage, not a schema change. + +The planner substitutes the two-step form: + +```sql +CREATE UNIQUE INDEX CONCURRENTLY "users_email_key" ON "public"."users" ("email"); +ALTER TABLE "public"."users" ADD CONSTRAINT "users_email_key" UNIQUE USING INDEX "users_email_key"; +``` + +Step 1 builds the index under `SHARE UPDATE EXCLUSIVE` — reads and writes +proceed throughout. Step 2 adopts the pre-built index as the constraint's +implementation under a brief, metadata-only `ACCESS EXCLUSIVE`. The engine +names the index after the constraint up front (your name is used as-is; a +generated name is built to fit PostgreSQL's identifier limit), so no rename +happens at adoption time. + +## Same end state, different path + +Catalog-wise the two forms are indistinguishable afterwards: both leave a +`pg_constraint` row of type `u` backed by a unique index of the same name, +and both validate **all existing rows** — neither grandfathers duplicates. +Everything that differs is on the way there: + +| | Submitted form (one statement) | Safer sequence (two steps) | +|---|---|---| +| Lock during the index build | `ACCESS EXCLUSIVE` for the whole scan + sort — blocks all reads and writes | `SHARE UPDATE EXCLUSIVE` — reads and writes proceed | +| Lock to attach the constraint | (included above) | `ACCESS EXCLUSIVE`, brief and metadata-only | +| A duplicate is found | Statement fails and rolls back cleanly — nothing left behind | The concurrent build fails and leaves an **INVALID index** that must be dropped before retrying — and an invalid *unique* index still enforces uniqueness against new writes until it is dropped | +| Transactionality | Can run inside a transaction block | `CREATE INDEX CONCURRENTLY` cannot — the sequence runs [autocommit-each-step](execution-model.md) | +| Cost | One table scan | Roughly two table scans plus waits for concurrent transactions to drain — slower in wall-clock time, cheaper in blocking | + +That third row is the trade in miniature: the safer sequence converts +*blocking* risk into *leftover-state* risk. pg-sprite takes that trade +deliberately — blocking is paid by every query on the table, leftover state +by one operator with a [documented recovery path](invalid-index-recovery.md) +— and reports the boundary precisely when it happens +([execution-model.md](execution-model.md)). + +## What the engine adds over running the sequence by hand + +The two-step form is a well-known idiom; the engine's job is everything +around it: + +- **Budgets on every step.** Brief catalog steps run under `SET LOCAL + lock_timeout` / `statement_timeout` in their own short transaction; the + concurrent build runs on a dedicated budgeted session. A lock pileup + cancels the step instead of queueing behind (and blocking everything + behind) a long-running query — the failure the raw `ADD CONSTRAINT … + USING INDEX` invites when run without a `lock_timeout`. +- **A typed verdict, not a scrollback.** Success or failure, the outcome + carries the committed prefix, the failed step, and a stable `code` — + the [execution model](execution-model.md) is the contract. +- **The substitution is visible before it runs.** `migrate --dry-run` + and `diff` print the sequence as compiler-style diagnostics; + the [plan report](plan-report.md) carries it as `safer_sql` with a typed + execution contract, so automation branches on fields, never on prose. + +## The substitutions the planner makes today + +| Submitted (blocking) form | Safer sequence | +|---|---| +| `ALTER COLUMN … SET NOT NULL` | 4 steps: `NOT VALID` CHECK → `VALIDATE` → `SET NOT NULL` (catalog flip) → drop the scaffold — [worked through step by step](execution-model.md#the-committed-prefix) | +| `ADD PRIMARY KEY` / `ADD UNIQUE` (direct) | 2 steps: `CREATE UNIQUE INDEX CONCURRENTLY` → `ADD CONSTRAINT … USING INDEX` (this page's example) | +| `ADD CHECK` / `ADD FOREIGN KEY` (direct) | 2 steps: `ADD CONSTRAINT … NOT VALID` → `VALIDATE CONSTRAINT` — the validation scan runs under a lock that lets reads and writes proceed | +| `CREATE INDEX` (non-concurrent) | 1 statement: the same build with `CONCURRENTLY` | +| `DETACH PARTITION` (non-concurrent) | 1 statement: `DETACH PARTITION … CONCURRENTLY` | + +Statements already in the safe form (`… USING INDEX`, `… NOT VALID`, +`CONCURRENTLY`) are recognized as the online idiom and run as submitted. +The full operation → lock → substitution matrix is +[postgres-online-ddl-reference.md](postgres-online-ddl-reference.md); the +machine-readable advisory shape is [suggest-report.md](suggest-report.md). + +## Caveats are typed, not prose + +A safer sequence is a *different way to run the change*, not a free +upgrade, so each substitution carries typed caveats in the +[suggest report](suggest-report.md#caveats-caveats) — `non-transactional`, +`separate-transactions`, `invalid-index-on-failure`, `validation-scan`, +`detach-finalize-on-failure` — that automation can branch on. The +`USING INDEX` form has structural limits of its own: the adopted index +must be a plain unique B-tree (not partial, not an expression index), and +`CREATE INDEX CONCURRENTLY` is not supported on partitioned tables — the +planner refuses with a typed reason +([`unsupported-partitioned-parent`](postgres-online-ddl-reference.md#unsupported-partitioned-parent)) +rather than substituting a sequence it cannot run online. From 299cd2ded0fb9cc1a105a4aa0537429fc14b4800 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Mon, 24 Aug 2026 11:09:08 +1000 Subject: [PATCH 3/3] docs: address both-lens review feedback on the safer-sequences page Adoption lens: add "check the claims yourself" and "when the substitution does not help" sections (home for the nullable-PK and DETACH-at-execution caveats). Seam lens: demote the substitution table to a labeled summary, teach readers to branch on the suggest report's fields instead, and pin every executor StepKind to docs/execution-model.md with a test. --- docs/execution-model.md | 29 ++++++---- docs/safer-sequences.md | 110 +++++++++++++++++++++++++++++++++----- pkg/executor/docs_test.go | 27 ++++++++++ pkg/executor/sequence.go | 6 +++ 4 files changed, 150 insertions(+), 22 deletions(-) create mode 100644 pkg/executor/docs_test.go diff --git a/docs/execution-model.md b/docs/execution-model.md index 9f0fc62..5c07367 100644 --- a/docs/execution-model.md +++ b/docs/execution-model.md @@ -52,16 +52,25 @@ the sequencing across transaction boundaries *is* the safety mechanism. "Implicit or bounded" above is the mechanical detail behind the contract — autocommit-each-step has two shapes in the executor: -- **Brief catalog steps and `VALIDATE CONSTRAINT`** each run as one short - *explicit* transaction: `BEGIN` → `SET LOCAL lock_timeout` / - `statement_timeout` → the statement → `COMMIT` (`pkg/executor`'s bounded - runner). The explicit `BEGIN` exists only because the budgets are applied - with `SET LOCAL`, which is scoped to that transaction — functionally it is - still one statement, one transaction, committed immediately, rolled back - atomically on failure. -- **`CREATE INDEX CONCURRENTLY`** is true autocommit on a dedicated budgeted - session: it refuses to run inside any transaction block and internally - manages multiple transactions of its own. +- **Brief catalog steps (step kind `brief`) and `VALIDATE CONSTRAINT` (step + kind `validate-constraint`)** each run as one short *explicit* transaction: + `BEGIN` → `SET LOCAL lock_timeout` / `statement_timeout` → the statement → + `COMMIT` (`pkg/executor`'s bounded runner). The explicit `BEGIN` exists + only because the budgets are applied with `SET LOCAL`, which is scoped to + that transaction — functionally it is still one statement, one + transaction, committed immediately, rolled back atomically on failure. +- **`CREATE INDEX CONCURRENTLY` (step kind `concurrent-index-build`)** is + true autocommit on a dedicated budgeted session: it refuses to run inside + any transaction block and internally manages multiple transactions of its + own. + +Each step's class is the `kind` field of its step report in the JSON +verdict — the field retry logic branches on. A failed `brief` step means +something held a lock longer than the brief budget tolerates: retrying is +reasonable. A failed `validate-constraint` step means the validation scan +exceeded its own budget: retrying without raising it will fail the same +way. A failed `concurrent-index-build` carries its own invalid-index +verdict ([invalid-index-recovery.md](invalid-index-recovery.md)). ## The committed prefix diff --git a/docs/safer-sequences.md b/docs/safer-sequences.md index 4e6df3e..6f7d2c3 100644 --- a/docs/safer-sequences.md +++ b/docs/safer-sequences.md @@ -5,7 +5,10 @@ PostgreSQL can do the work without a table rewrite, but the form as written holds `ACCESS EXCLUSIVE` while it does real work — and substitutes the **safer native sequence**: an ordered set of statements that reaches the same declared end state while confining every exclusive lock to a brief, -metadata-only catalog flip. The classification is `safer-idiom` in the +metadata-only catalog flip (the exception — `ADD PRIMARY KEY` on a nullable +column — and the other boundaries are in +[When the substitution does not help](#when-the-substitution-does-not-help)). +The classification is `safer-idiom` in the [plan report](plan-report.md); `migrate` executes the substitution, `migrate --dry-run` and [`suggest`](suggest-report.md) show it, and [`lint`](lint-report.md) flags the blocking form offline. Watch the whole @@ -17,6 +20,8 @@ flow in [demos/improve.gif](demos/improve.gif). - [Same end state, different path](#same-end-state-different-path) - [What the engine adds over running the sequence by hand](#what-the-engine-adds-over-running-the-sequence-by-hand) - [The substitutions the planner makes today](#the-substitutions-the-planner-makes-today) +- [When the substitution does not help](#when-the-substitution-does-not-help) +- [Check the claims yourself](#check-the-claims-yourself) - [Caveats are typed, not prose](#caveats-are-typed-not-prose) ## A worked example: `ADD CONSTRAINT … UNIQUE` @@ -89,19 +94,106 @@ around it: ## The substitutions the planner makes today +An at-a-glance summary — the authoritative, per-statement answer comes from +the tool itself (see below): + | Submitted (blocking) form | Safer sequence | |---|---| | `ALTER COLUMN … SET NOT NULL` | 4 steps: `NOT VALID` CHECK → `VALIDATE` → `SET NOT NULL` (catalog flip) → drop the scaffold — [worked through step by step](execution-model.md#the-committed-prefix) | -| `ADD PRIMARY KEY` / `ADD UNIQUE` (direct) | 2 steps: `CREATE UNIQUE INDEX CONCURRENTLY` → `ADD CONSTRAINT … USING INDEX` (this page's example) | +| `ADD UNIQUE` (direct) | 2 steps: `CREATE UNIQUE INDEX CONCURRENTLY` → `ADD CONSTRAINT … USING INDEX` (this page's example) | +| `ADD PRIMARY KEY` (direct) | The same 2 steps — **on a column already `NOT NULL`**. On a nullable column, adopting the index must also set `NOT NULL`, and PostgreSQL validates that by scanning the heap under `ACCESS EXCLUSIVE` — the blocking work the substitution exists to avoid, and a scan the brief step budget cancels. Reach `NOT NULL` first via the 4-step sequence above, then add the primary key | | `ADD CHECK` / `ADD FOREIGN KEY` (direct) | 2 steps: `ADD CONSTRAINT … NOT VALID` → `VALIDATE CONSTRAINT` — the validation scan runs under a lock that lets reads and writes proceed | | `CREATE INDEX` (non-concurrent) | 1 statement: the same build with `CONCURRENTLY` | -| `DETACH PARTITION` (non-concurrent) | 1 statement: `DETACH PARTITION … CONCURRENTLY` | +| `DETACH PARTITION` (non-concurrent) | 1 statement: `DETACH PARTITION … CONCURRENTLY` — shown by `--dry-run` and `suggest`, but **execution refuses this step today**: a cancelled concurrent detach leaves a detach-pending partition state the executor does not own recovering | Statements already in the safe form (`… USING INDEX`, `… NOT VALID`, `CONCURRENTLY`) are recognized as the online idiom and run as submitted. The full operation → lock → substitution matrix is -[postgres-online-ddl-reference.md](postgres-online-ddl-reference.md); the -machine-readable advisory shape is [suggest-report.md](suggest-report.md). +[postgres-online-ddl-reference.md](postgres-online-ddl-reference.md). + +The table is a summary; the tool is the source of truth. For any statement, +ask it directly — offline, nothing executes: + +```console +$ echo 'ALTER TABLE users ADD CONSTRAINT users_email_key UNIQUE (email);' \ + | pg-sprite suggest --json +``` + +```json +{ + "format_version": 2, + "suggestions": [ + { + "statement": 1, + "line": 1, + "column": 1, + "original": "ALTER TABLE users ADD CONSTRAINT users_email_key UNIQUE (email)", + "operation": "ADD CONSTRAINT users_email_key", + "reason": "safer-idiom", + "recommended": [ + "CREATE UNIQUE INDEX CONCURRENTLY \"users_email_key\" ON \"users\" (\"email\")", + "ALTER TABLE \"users\" ADD CONSTRAINT \"users_email_key\" UNIQUE USING INDEX \"users_email_key\"" + ], + "execution": "autocommit-each-step", + "caveats": ["non-transactional", "separate-transactions", "invalid-index-on-failure"] + } + ] +} +``` + +`recommended` carries the substituted sequence, `caveats` the typed +conditions of running it — branch on those fields. (Offline, names render +as submitted; execution resolves them, which is why the worked example +above reads `"public"."users"`.) The versioned shape is the +[suggest report contract](suggest-report.md), so a new substitution cannot +ship with a stale doc row as its only description. + +## When the substitution does not help + +The substitution covers statements that are native but blocking as written. +It does not cover: + +- **Operations that need a table rewrite** — a column type change that + PostgreSQL cannot convert in place, for example. No safer native sequence + exists; that is copy-and-swap's job. The per-operation routing is in + [postgres-online-ddl-reference.md](postgres-online-ddl-reference.md). +- **`ADD PRIMARY KEY` on a nullable column.** Adopting the pre-built index + must also set `NOT NULL`, which PostgreSQL validates by scanning the heap + under `ACCESS EXCLUSIVE` — so the adoption step is not the brief catalog + flip it is everywhere else, and the brief step budget cancels it. Reach + `NOT NULL` first via its own 4-step sequence, then add the primary key. +- **The `USING INDEX` structural limits.** The adopted index must be a + plain unique B-tree — not partial, not an expression index. And + `CREATE INDEX CONCURRENTLY` is not supported on partitioned tables — + pg-sprite refuses with a typed reason + ([`unsupported-partitioned-parent`](postgres-online-ddl-reference.md#unsupported-partitioned-parent)) + rather than substituting a sequence it cannot run online. +- **`DETACH PARTITION … CONCURRENTLY` at execution.** The planner + substitutes it and `--dry-run`/`suggest` show it, but the executor + refuses to run the step: a cancelled concurrent detach leaves a + detach-pending partition state it does not own recovering. + +## Check the claims yourself + +Every lock and duration this page claims is observable on your own table. +While a sequence runs, watch the locks from another session — during step 1 +and again during step 2: + +```sql +SELECT mode, granted FROM pg_locks WHERE relation = 'users'::regclass; +``` + +and time the steps (`\timing on` in psql), or watch who waits on whom: + +```sql +SELECT query, state, wait_event_type FROM pg_stat_activity +WHERE query LIKE '%users%'; +``` + +Run the same checks against the submitted form on a scratch copy and the +comparison table above reproduces itself: the one-statement form holds +`ACCESS EXCLUSIVE` for the whole build; the two-step form never holds it +longer than a catalog flip. ## Caveats are typed, not prose @@ -109,10 +201,4 @@ A safer sequence is a *different way to run the change*, not a free upgrade, so each substitution carries typed caveats in the [suggest report](suggest-report.md#caveats-caveats) — `non-transactional`, `separate-transactions`, `invalid-index-on-failure`, `validation-scan`, -`detach-finalize-on-failure` — that automation can branch on. The -`USING INDEX` form has structural limits of its own: the adopted index -must be a plain unique B-tree (not partial, not an expression index), and -`CREATE INDEX CONCURRENTLY` is not supported on partitioned tables — the -planner refuses with a typed reason -([`unsupported-partitioned-parent`](postgres-online-ddl-reference.md#unsupported-partitioned-parent)) -rather than substituting a sequence it cannot run online. +`detach-finalize-on-failure` — that automation can branch on. diff --git a/pkg/executor/docs_test.go b/pkg/executor/docs_test.go new file mode 100644 index 0000000..8c3bd13 --- /dev/null +++ b/pkg/executor/docs_test.go @@ -0,0 +1,27 @@ +package executor_test + +import ( + "fmt" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/executor" +) + +// executionModelDoc is the human-facing contract page this test keeps honest. +const executionModelDoc = "../../docs/execution-model.md" + +// Every step kind automation can branch on must be named in the execution +// model: a StepKind added to the code without the doc naming it fails here. +func TestDocNamesEveryStepKind(t *testing.T) { + raw, err := os.ReadFile(executionModelDoc) + require.NoError(t, err) + doc := string(raw) + for _, k := range executor.StepKinds() { + assert.Contains(t, doc, fmt.Sprintf("`%s`", k), + "docs/execution-model.md does not name step kind %q", k) + } +} diff --git a/pkg/executor/sequence.go b/pkg/executor/sequence.go index 2c967cf..f719a23 100644 --- a/pkg/executor/sequence.go +++ b/pkg/executor/sequence.go @@ -77,6 +77,12 @@ const ( StepValidateConstraint StepKind = "validate-constraint" ) +// StepKinds returns the closed set of StepKind values. It is part of the +// documented contract: docs/execution-model.md names every kind. +func StepKinds() []StepKind { + return []StepKind{StepBrief, StepConcurrentIndexBuild, StepValidateConstraint} +} + // ValidateBudget bounds one VALIDATE CONSTRAINT step. The validation scan // is long by design — it is the online half of the NOT VALID pattern — so // it gets its own overall bound instead of the brief statement budget,