diff --git a/README.md b/README.md index 2804943..0c1dfe9 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,10 @@ refusal โ€” never a silently wrong or incomplete result: tooling, then state it in the file. A serial cannot state one, so export refuses a serial whose sequence was declared otherwise. - **Partitioned tables** support in-place statement changes, but cannot be - expressed in or exported to desired files. + expressed in or exported to desired files, and `migrate` refuses + `ATTACH PARTITION` and `DETACH PARTITION`. +- **Index maintenance** is not driven: `migrate` refuses `DROP INDEX` and + `REINDEX` (plain or `CONCURRENTLY`); see the `--accept-blocking` note below. - **Unlogged tables and explicit column collations** are outside the declarative model: converging either is a table (or column) rewrite, so export and diff refuse rather than plan one. diff --git a/docs/capabilities-contract.md b/docs/capabilities-contract.md index be4ba21..2ca4138 100644 --- a/docs/capabilities-contract.md +++ b/docs/capabilities-contract.md @@ -33,7 +33,8 @@ become `area`; the first column is named `operation`, `table_shape`, or - `status_mark`: exactly `โœ…`, `๐ŸŸก`, `โšช`, `๐Ÿ”ต`, or `โŒ`. They mean, respectively, supported today, planned with a typed refusal today, out of scope because there is no online-safety problem, out of scope because another tool class owns the work, and - out of scope because PostgreSQL has no online mechanism. + out of scope because there is no online mechanism pg-sprite will drive: PostgreSQL + has none, or the only online form is a direct statement the operator runs. - `tier`: `t1`, `t2`, or `t3`, derived consistently from the mark but stored and validated so a consumer need not understand emoji. `โœ…` is T1, `๐ŸŸก` is T2, and all three remaining marks are T3. @@ -214,9 +215,13 @@ row and a consumer reading a verdict reach the same route: | T2 / `๐ŸŸก` | `capability-boundary` | | T3 / `โšช` | `no-online-safety-problem`, with `owner: direct-operator` | | T3 / `๐Ÿ”ต` | `no-online-safety-problem`, with `owner` naming the tool class | -| T3 / `โŒ` | `by-design` | +| T3 / `โŒ` | `by-design` (see below for index maintenance) | -T1 / `โœ…` has no capability refusal class. Two classes have no matrix row: `environmental` +The `โŒ` rows for index maintenance (`drop-index`, `reindex`) span two classes: the +plain form refuses as `by-design`, while the concurrent form, which is the direct +statement the operator runs, refuses as `no-online-safety-problem` with +`owner: direct-operator`. Both carry the row's `index-statement` reason. T1 / `โœ…` has +no capability refusal class. Two classes have no matrix row: `environmental` describes the run site โ€” privileges, contention, budgets, or other conditions โ€” not whether pg-sprite supports an operation, and `invariant-violation` reports a defect in pg-sprite, not a property of the operation. The matrix and that contract must change together. diff --git a/docs/capabilities.md b/docs/capabilities.md index ae8bf45..341cdbb 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -57,8 +57,9 @@ pg-sprite capabilities --json | jq '.capabilities[] | select(.engine_path == "co # in diff, but can execute through migrate --desired. pg-sprite capabilities --json | jq '.capabilities[] | select(.front_doors.diff == "refused")' -# Rows another tool class owns: the โšช and ๐Ÿ”ต rows. The โŒ rows name no owner, because -# PostgreSQL offers no online mechanism for them, so this is not the full out-of-scope set. +# Rows another tool class owns: the โšช and ๐Ÿ”ต rows. The โŒ rows name no owner: they have +# no online mechanism pg-sprite will drive (PostgreSQL has none, or the only online form +# is a direct statement the operator runs), so this is not the full out-of-scope set. pg-sprite capabilities --json | jq '.capabilities[] | select(.owning_tool_class != null)' ``` @@ -96,9 +97,9 @@ Two consequences follow, and they explain most of this page: | Tier | Meaning | What you see today | | --- | --- | --- | -| **T1 โ€” supported today** | The engine executes the change through an online-safe pattern | Execution (or the safer rewritten sequence), exit 0; where `migrate` refuses a plain index-maintenance form and names its `CONCURRENTLY` idiom, `--accept-blocking` runs the plain form under budgets and exits 3 | +| **T1 โ€” supported today** | The engine executes the change through an online-safe pattern | Execution (or the safer rewritten sequence), exit 0 | | **T2 โ€” planned** | A known online pattern exists (or requires the copy-and-swap engine); building it is on the roadmap | A **typed refusal** naming the reason, exit 2 โ€” never a silent fallback to a blocking form | -| **T3 โ€” out of scope by design** | No online-safety problem to solve, solving it belongs to a different tool class, or PostgreSQL offers no online mechanism to build on | A typed refusal or a parse-level rejection, with the reason stating *why it is not planned* | +| **T3 โ€” out of scope by design** | No online-safety problem to solve, solving it belongs to a different tool class, or there is no online mechanism pg-sprite will drive (PostgreSQL has none, or the only online form is a direct statement the operator runs) | A typed refusal or a parse-level rejection, with the reason stating *why it is not planned* | T3 rows carry one of three marks, because they mean different things โ€” and only one of them is a limitation: @@ -112,9 +113,12 @@ them is a limitation: - **๐Ÿ”ต a different tool class owns it** โ€” the job is real but belongs to another kind of tool (data-change runners, provisioning/IaC, convergence planners, expand/contract frameworks). The row's "Online-safety problem?" column names the class to look for. -- **โŒ no online mechanism exists** โ€” PostgreSQL itself provides no online pattern to - build on, so pg-sprite refuses rather than silently run the blocking form. These are - the only rows where "unsupported" is the honest reading. +- **โŒ no online mechanism pg-sprite will drive** โ€” PostgreSQL has none, or the only + online form is a direct statement the operator runs, so pg-sprite refuses rather than + silently run the blocking form. Index maintenance (`DROP INDEX`, `REINDEX`) is the + second case: the online form is a plain `CONCURRENTLY` statement, which `migrate` + refuses to wrap and names so you run it directly; for a single-relation plain form, + `--accept-blocking` runs it under budgets and exits 3. Every matrix table carries an **"Online-safety problem?"** column: "Yes" means there is a readers-and-writers problem for an online engine to solve (pg-sprite solves it, plans to, @@ -151,7 +155,8 @@ engine: shadow-copy engine; a typed refusal until that engine lands (see [optimistic-attempt.md](optimistic-attempt.md) for how the router proves the rewrite). - **โ€”** โ€” no engine path: either the job belongs to another tool class (โšช / ๐Ÿ”ต) or - PostgreSQL offers no online mechanism to build on (โŒ). + there is no online mechanism pg-sprite will drive (โŒ): PostgreSQL has none, or the + only online form is a direct statement the operator runs. Path and tier are orthogonal on purpose: a ๐ŸŸก row's path says what kind of work lifts the refusal โ€” building a native flow or landing the copy engine โ€” and a โœ… row's path @@ -182,15 +187,16 @@ are the canonical example. > `make gen-capabilities`; do not edit the generated regions below by hand. -**60 operations: 20 supported today, 24 planned behind a typed refusal, 14 out of scope -by design, and 2 with no online mechanism in PostgreSQL to build on.** +**60 operations: 16 supported today, 26 planned behind a typed refusal, 14 out of scope +by design, and 4 with no online mechanism for pg-sprite to build on.** Status legend: โœ… T1 (supported today) ยท ๐ŸŸก T2 (planned; typed refusal today) ยท โšช T3 (out of scope; **no online-safety problem** โ€” run directly, or through whatever review the object warrants) ยท ๐Ÿ”ต T3 (out of scope; **a different tool class owns it**) ยท -โŒ T3 (out of scope; **no online mechanism exists** in PostgreSQL). +โŒ T3 (out of scope; **no online mechanism pg-sprite will drive**: PostgreSQL has none, +or the only online form is a direct statement the operator runs). ### Column changes @@ -231,8 +237,8 @@ review the object warrants) ยท | Operation | Status | Engine path | Online-safety problem? | Behavior and why | | --- | --- | --- | --- | --- | | `CREATE [UNIQUE] INDEX` on a plain table โ€” including partial, expression, covering (`INCLUDE`), GIN/GiST/BRIN | โœ… | native, safer sequence | Yes | Executed as (or rewritten to) `CREATE INDEX CONCURRENTLY`, with validity verification, typed invalid-index outcomes, and a proven recovery for abandoned leftovers (`RebuildAbandonedIndex`, or `DropAbandonedIndex` to remove the leftover without rebuilding; both library-only; [runbook](invalid-index-recovery.md)) | -| `DROP INDEX` | โœ… | native, safer sequence | Yes | Rewritten to `DROP INDEX CONCURRENTLY`; flagged **destructive**. `migrate` refuses the plain form and names the idiom; a single-index `DROP INDEX` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0). `diff` refuses dropping the table's replica identity index, or one a partition's is attached under, with `replica-identity`. `migrate` refuses every plain `DROP INDEX` before it reads the table, so its dry-run names the concurrent idiom; `--accept-blocking` checks the replica identity under the table's lock and refuses the drop as `unsupported-statement`, naming the identity index. pg-sprite never runs the concurrent drop itself | -| `REINDEX` | โœ… | native, safer sequence | Yes | Rewritten to `REINDEX ... CONCURRENTLY`. `migrate` refuses the plain form and names the idiom; a plain `REINDEX INDEX` or `REINDEX TABLE` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0). `REINDEX SCHEMA`, `DATABASE`, and `SYSTEM` stay refused | +| `DROP INDEX` | โŒ | โ€” | Yes โ€” the online form is a direct `CONCURRENTLY` statement that `migrate` does not drive | `migrate` never runs this statement online-safely: the plain form is refused and names `DROP INDEX CONCURRENTLY`, and the concurrent form is refused as already safe โ€” run it directly against the database. A single-index plain `DROP INDEX` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0); `--accept-blocking` checks the replica identity under the table's lock and refuses the drop as `unsupported-statement`, naming the identity index. `diff` reports an index to remove as a **destructive** `DROP INDEX` and hints to run it through `migrate --alter`, which ends in this `index-statement` refusal โ€” run `DROP INDEX CONCURRENTLY` directly instead. `diff` refuses dropping the table's replica identity index, or one a partition's is attached under, with `replica-identity` | +| `REINDEX` | โŒ | โ€” | Yes โ€” the online form is a direct `CONCURRENTLY` statement that `migrate` does not drive | `migrate` never runs this statement online-safely: the plain form is refused and names `REINDEX ... CONCURRENTLY`, and the concurrent form is refused as already safe โ€” run it directly against the database. A plain `REINDEX INDEX` or `REINDEX TABLE` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0). `REINDEX SCHEMA`, `DATABASE`, and `SYSTEM` stay refused. A desired schema cannot express a reindex, so `diff` never produces one | | Index build on a **partitioned parent** | ๐ŸŸก | native, planned flow | Yes | PostgreSQL has no parent-level `CONCURRENTLY`; the blocking form is refused by policy (`--force` does not bypass it, and `--accept-blocking` does not reach it yet). The partition-aware flow โ€” `CREATE INDEX ON ONLY` โ†’ per-partition CIC โ†’ `ATTACH PARTITION`, with crash-resume per leaf โ€” is planned | | `ADD CONSTRAINT ... USING INDEX` on a partitioned parent | โŒ | โ€” | Yes โ€” unsolvable today | PostgreSQL does not support adopting an index on a partitioned parent in any supported version; refused before execution | @@ -243,8 +249,8 @@ review the object warrants) ยท | Operation | Status | Engine path | Online-safety problem? | Behavior and why | | --- | --- | --- | --- | --- | | `CREATE TABLE ... PARTITION OF` | ๐ŸŸก | native, planned flow | Yes | Typed refusal at both doors: the imperative door does not take `CREATE TABLE`, and the declarative create path refuses the form at plan time and re-checks it at apply โ€” attaching a partition takes a brief `ACCESS EXCLUSIVE` on the **parent**, which the greenfield absence proof does not cover. A desired file for a partition that already exists is refused before planning builds anything, since the scratch build would attach to the live parent. The partition-aware flow is planned | -| `ATTACH PARTITION` | โœ… | native, as-is | Yes | Executed; the safer idiom (pre-prove the bound with a validated `CHECK` so the attach skips its scan) is surfaced as guidance. A classify-first flow that constructs the proof itself is planned | -| `DETACH PARTITION [CONCURRENTLY]` | โœ… | native, safer sequence | Yes | `CONCURRENTLY` is the idiom; the blocking form is rewritten to it | +| `ATTACH PARTITION` | ๐ŸŸก | native, planned flow | Yes | Without `--force`, a typed refusal with nothing executed: the attach scans the partition to validate its bound unless a validated `CHECK` already proves it, and the engine does not yet construct that proof. The safer idiom (pre-prove the bound with a validated `CHECK` so the attach skips its scan) is surfaced as guidance. `--force SCHEMA.TABLE` runs the blocking `ATTACH PARTITION` as submitted under bounded budgets, taking the blocking lock, and records the override. The classify-first flow that constructs the proof itself is planned. A desired schema cannot express an attach, so `diff` never produces one | +| `DETACH PARTITION [CONCURRENTLY]` | ๐ŸŸก | native, planned flow | Yes | `CONCURRENTLY` is the idiom and the blocking form is rewritten to it (`--dry-run` and `suggest` show the rewrite), but without `--force` `migrate` refuses both forms before anything executes: a cancelled concurrent detach leaves a detach-pending partition state the executor does not yet own recovering. `--force SCHEMA.TABLE` runs a plain `DETACH PARTITION` as submitted under bounded budgets, taking the blocking lock; `DETACH ... CONCURRENTLY` stays refused under `--force`. `migrate --dry-run` of either form reports the rewrite as a safer sequence to run and exits 0 while the real run refuses and exits 2, so the dry-run exit code is not a submit gate for a detach. A desired schema cannot express a detach, so `diff` never produces one | | Partitioned parents in the **declarative model** | ๐ŸŸก | native, planned flow | Yes | Typed refusal: the model does not yet carry partition keys, and rendering a partitioned parent as a plain `CREATE TABLE` would be silently wrong | | Partitioned tables in **copy-and-swap** | ๐ŸŸก | copy-and-swap | Yes | Root-vs-leaf publication semantics and per-partition swap; sequenced after the copy engine core | @@ -274,7 +280,7 @@ review the object warrants) ยท | --- | --- | --- | --- | --- | | Enum-typed columns on plain tables | ๐ŸŸก | native, planned flow | Yes | Tolerance end to end (introspection already canonicalizes via `format_type`; desired-file admission and transaction-scoped scratch-schema mechanics are being verified) | | `ALTER TYPE ... ADD VALUE` | ๐ŸŸก | native, planned flow | Yes | Metadata-only and online-safe (PG 14+ allows it in a transaction; the value is usable after commit) โ€” planned as an owned operation. No peer online executor owns it | -| Enum value rename / removal | ๐ŸŸก | copy-and-swap | Yes | PostgreSQL has no `DROP VALUE`; this is a type swap + table rewrite โ€” routes to a typed refusal toward copy-and-swap | +| Enum value rename / removal | ๐ŸŸก | copy-and-swap | Yes | PostgreSQL has no `DROP VALUE`; this is a type swap + table rewrite, planned on copy-and-swap. Today the imperative door refuses `ALTER TYPE ... RENAME VALUE` as `unsupported-statement`, since it takes only `ALTER TABLE` and `CREATE INDEX`, and `DROP VALUE` does not parse | | Enum/domain type creation and drop | โšช | โ€” | No โ€” owner tooling (psql, shipped with the code change) | Bootstrap/catalog work with no concurrent-access problem; owner tooling applies it in the same change that ships the code | | Views, materialized views (create and replace) | โšช | โ€” | No โ€” owner tooling | Transactional catalog work, but `CREATE OR REPLACE VIEW` takes a brief `ACCESS EXCLUSIVE` on the view and queues behind in-flight readers โ€” run it under a `lock_timeout` | | `REFRESH MATERIALIZED VIEW` | ๐Ÿ”ต | โ€” | No โ€” data jobs / owner tooling | A data operation, not catalog work: the plain form holds `ACCESS EXCLUSIVE` on the matview for the whole rebuild (`CONCURRENTLY` needs a unique index and trades the lock for churn). Scheduling refreshes belongs to data jobs | @@ -430,8 +436,8 @@ Three related jobs stay with humans on purpose: the recovery handles, and what the rest license a human to do is [invalid-index-recovery.md](invalid-index-recovery.md). - **Index maintenance (`REINDEX` automation, bloat-driven rebuild scheduling).** The - engine executes `REINDEX CONCURRENTLY` when asked (see matrix); *deciding* when an - index needs rebuilding is monitoring-and-operations territory. Automating it is + engine does not drive `REINDEX` at all (see matrix) โ€” run `REINDEX ... CONCURRENTLY` + directly; *deciding* when an index needs rebuilding is monitoring-and-operations territory. Automating it is considered but on hold for the same ownership reasons as above. --- diff --git a/docs/limitations.md b/docs/limitations.md index 9f44f6c..5b806e1 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -14,6 +14,9 @@ not escape hatches: | `ADD CONSTRAINT ... USING INDEX` on a partitioned parent | PostgreSQL does not support adopting an existing index on a partitioned parent in any supported version. pg-sprite refuses before execution. | | `ADD PRIMARY KEY` / `ADD UNIQUE` with a storage parameter the online sequence cannot render | The online sequence builds the key's index with `CREATE UNIQUE INDEX CONCURRENTLY` and carries the key's `WITH (...)` parameters onto it, each name quoted and each value a quoted literal. A parameter it cannot render faithfully, one whose value is not a plain word, number, or string, contains a backslash, or whose name is namespaced, would be dropped or changed, so no sequence is built. Running the statement as written builds the index under `ACCESS EXCLUSIVE`, so it is refused as `rewrite-required` with the `unique-index-then-constraint` guidance. Build the index concurrently with the parameter, then attach it with `ADD CONSTRAINT ... USING INDEX`. | | `ADD FOREIGN KEY ... NOT VALID` on a partitioned parent | PostgreSQL does not support this before version 18, so pg-sprite refuses it on versions 14โ€“17. It is supported on version 18 and later. | +| `DROP INDEX` and `REINDEX`, plain or `CONCURRENTLY` | `migrate` never runs index maintenance online-safely. The plain form is refused with `index-statement` and names its `CONCURRENTLY` idiom; the concurrent form is refused as already safe, so run it directly against the database. `migrate --accept-blocking SCHEMA.TABLE` runs a single-relation plain `DROP INDEX`, `REINDEX INDEX`, or `REINDEX TABLE` as-is under bounded budgets and exits 3 ([lock-budgeted-passthrough.md](lock-budgeted-passthrough.md)). `diff`'s hint to run a removed index through `migrate --alter` ends in this refusal; run `DROP INDEX CONCURRENTLY` directly. | +| `ATTACH PARTITION` | Without `--force`, refused with `not-native-safe-rewrite-required`, nothing executed: the engine does not yet construct the validated `CHECK` that lets the attach skip its scan. Pre-prove the bound yourself, then run the attach directly. `--force SCHEMA.TABLE` runs the blocking `ATTACH PARTITION` as submitted under bounded budgets, taking the blocking lock. | +| `DETACH PARTITION` and `DETACH PARTITION ... CONCURRENTLY` | Without `--force`, refused with `unsupported-statement` before anything executes: the plain form is rewritten to `CONCURRENTLY`, and the executor does not drive that step because a cancelled concurrent detach leaves a detach-pending state it does not yet own recovering. Run `DETACH PARTITION ... CONCURRENTLY` directly. `--force SCHEMA.TABLE` runs a plain `DETACH PARTITION` as submitted under bounded budgets, taking the blocking lock; `DETACH ... CONCURRENTLY` stays refused under `--force`. `migrate --dry-run` reports the rewrite as a step to run and exits 0 while the real run refuses (exit 2), so the dry-run exit code is not a submit gate for a detach. | | Dropping what a replica identity index is built on | `REPLICA IDENTITY USING INDEX` outlives its index, so dropping the index, the primary key or unique constraint that owns it, or a column it covers leaves the table with no usable identity, and every `UPDATE` and `DELETE` on it fails while a publication publishes them. The planner refuses such a drop with `replica-identity`, on the table and on what the drop reaches below it: a `DROP COLUMN` reaches partitions and inheritance children unless the statement says `ONLY`, and a `DROP CONSTRAINT` reaches partitions, whose indexes are attached under the parent's. The executor reads the identity again under the lock the drop takes, so an identity that moved onto the dropped object after planning still refuses the run before anything commits. `migrate --accept-blocking` makes the same check before a `DROP INDEX`. Point `REPLICA IDENTITY` at another unique index first, or `DEFAULT` if a usable primary key remains after the drop, then re-plan. | | `ADD COLUMN` on a hot standby, or by a role without `TEMPORARY` | Whether adding a column rewrites the table is the server's answer, read by adding the column to an empty temporary table. A server that accepts no writes, or a role whose `TEMPORARY` on the database was revoked, cannot create that table, so the plan fails with `ErrAddColumnProbeUnavailable` before anything runs. Plan against the primary, or grant `TEMPORARY` on the database to the engine's role. | | Copy-and-swap | The copy-and-swap backend is not yet available. Statements that require it route to `refuse`; pg-sprite never falls through to a blocking rewrite. | diff --git a/pkg/capabilities/capabilities.go b/pkg/capabilities/capabilities.go index fd8f6bb..954284e 100644 --- a/pkg/capabilities/capabilities.go +++ b/pkg/capabilities/capabilities.go @@ -224,7 +224,7 @@ func RenderDocument(input []byte, rows []Row) ([]byte, error) { counts[r.StatusMark]++ tiers[r.Tier]++ } - summary := fmt.Sprintf("**%d operations: %d supported today, %d planned behind a typed refusal, %d out of scope\nby design, and %d with no online mechanism in PostgreSQL to build on.**", len(rows), tiers[TierOne], tiers[TierTwo], counts[StatusNoSafetyProblem]+counts[StatusOtherTool], counts[StatusNoOnlineMechanism]) + summary := fmt.Sprintf("**%d operations: %d supported today, %d planned behind a typed refusal, %d out of scope\nby design, and %d with no online mechanism for pg-sprite to build on.**", len(rows), tiers[TierOne], tiers[TierTwo], counts[StatusNoSafetyProblem]+counts[StatusOtherTool], counts[StatusNoOnlineMechanism]) var err error out, err = replaceRegion(out, "summary", summary) if err != nil { diff --git a/pkg/capabilities/capabilities.yaml b/pkg/capabilities/capabilities.yaml index 0e5b715..f70c418 100644 --- a/pkg/capabilities/capabilities.yaml +++ b/pkg/capabilities/capabilities.yaml @@ -68,7 +68,7 @@ rows: front_doors: migrate: refused diff: refused - refusal_reason: unsupported-statement + refusal_reason: not-native-safe-rewrite-required reason_notes: "The inline constraint does its index build or validation scan under the `ADD COLUMN`'s `ACCESS EXCLUSIVE` lock; refused with guidance to add the column first, then build the constraint online" - id: drop-column area: "column_changes" @@ -232,25 +232,29 @@ rows: - id: drop-index area: "indexes" operation: "`DROP INDEX`" - tier: "t1" - status_mark: "โœ…" - engine_path: "native_safer_sequence" + tier: "t3" + status_mark: "โŒ" + engine_path: "none" online_safety_problem: true + online_safety_detail: "the online form is a direct `CONCURRENTLY` statement that `migrate` does not drive" front_doors: - migrate: supported + migrate: refused diff: supported - reason_notes: "Rewritten to `DROP INDEX CONCURRENTLY`; flagged **destructive**. `migrate` refuses the plain form and names the idiom; a single-index `DROP INDEX` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0). `diff` refuses dropping the table's replica identity index, or one a partition's is attached under, with `replica-identity`. `migrate` refuses every plain `DROP INDEX` before it reads the table, so its dry-run names the concurrent idiom; `--accept-blocking` checks the replica identity under the table's lock and refuses the drop as `unsupported-statement`, naming the identity index. pg-sprite never runs the concurrent drop itself" + refusal_reason: index-statement + reason_notes: "`migrate` never runs this statement online-safely: the plain form is refused and names `DROP INDEX CONCURRENTLY`, and the concurrent form is refused as already safe โ€” run it directly against the database. A single-index plain `DROP INDEX` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0); `--accept-blocking` checks the replica identity under the table's lock and refuses the drop as `unsupported-statement`, naming the identity index. `diff` reports an index to remove as a **destructive** `DROP INDEX` and hints to run it through `migrate --alter`, which ends in this `index-statement` refusal โ€” run `DROP INDEX CONCURRENTLY` directly instead. `diff` refuses dropping the table's replica identity index, or one a partition's is attached under, with `replica-identity`" - id: reindex area: "indexes" operation: "`REINDEX`" - tier: "t1" - status_mark: "โœ…" - engine_path: "native_safer_sequence" + tier: "t3" + status_mark: "โŒ" + engine_path: "none" online_safety_problem: true + online_safety_detail: "the online form is a direct `CONCURRENTLY` statement that `migrate` does not drive" front_doors: - migrate: supported - diff: supported - reason_notes: "Rewritten to `REINDEX ... CONCURRENTLY`. `migrate` refuses the plain form and names the idiom; a plain `REINDEX INDEX` or `REINDEX TABLE` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0). `REINDEX SCHEMA`, `DATABASE`, and `SYSTEM` stay refused" + migrate: refused + diff: not_applicable + refusal_reason: index-statement + reason_notes: "`migrate` never runs this statement online-safely: the plain form is refused and names `REINDEX ... CONCURRENTLY`, and the concurrent form is refused as already safe โ€” run it directly against the database. A plain `REINDEX INDEX` or `REINDEX TABLE` can be run as-is under bounded budgets with `--accept-blocking SCHEMA.TABLE` (exit 3, never 0). `REINDEX SCHEMA`, `DATABASE`, and `SYSTEM` stay refused. A desired schema cannot express a reindex, so `diff` never produces one" - id: index-build-on-a-partitioned-parent area: "indexes" operation: "Index build on a **partitioned parent**" @@ -291,25 +295,27 @@ rows: - id: attach-partition area: "partitioned_tables" operation: "`ATTACH PARTITION`" - tier: "t1" - status_mark: "โœ…" - engine_path: "native_as_is" + tier: "t2" + status_mark: "๐ŸŸก" + engine_path: "native_planned_flow" online_safety_problem: true front_doors: - migrate: supported - diff: supported - reason_notes: "Executed; the safer idiom (pre-prove the bound with a validated `CHECK` so the attach skips its scan) is surfaced as guidance. A classify-first flow that constructs the proof itself is planned" + migrate: refused + diff: not_applicable + refusal_reason: not-native-safe-rewrite-required + reason_notes: "Without `--force`, a typed refusal with nothing executed: the attach scans the partition to validate its bound unless a validated `CHECK` already proves it, and the engine does not yet construct that proof. The safer idiom (pre-prove the bound with a validated `CHECK` so the attach skips its scan) is surfaced as guidance. `--force SCHEMA.TABLE` runs the blocking `ATTACH PARTITION` as submitted under bounded budgets, taking the blocking lock, and records the override. The classify-first flow that constructs the proof itself is planned. A desired schema cannot express an attach, so `diff` never produces one" - id: detach-partition-concurrently area: "partitioned_tables" operation: "`DETACH PARTITION [CONCURRENTLY]`" - tier: "t1" - status_mark: "โœ…" - engine_path: "native_safer_sequence" + tier: "t2" + status_mark: "๐ŸŸก" + engine_path: "native_planned_flow" online_safety_problem: true front_doors: - migrate: supported - diff: supported - reason_notes: "`CONCURRENTLY` is the idiom; the blocking form is rewritten to it" + migrate: refused + diff: not_applicable + refusal_reason: unsupported-statement + reason_notes: "`CONCURRENTLY` is the idiom and the blocking form is rewritten to it (`--dry-run` and `suggest` show the rewrite), but without `--force` `migrate` refuses both forms before anything executes: a cancelled concurrent detach leaves a detach-pending partition state the executor does not yet own recovering. `--force SCHEMA.TABLE` runs a plain `DETACH PARTITION` as submitted under bounded budgets, taking the blocking lock; `DETACH ... CONCURRENTLY` stays refused under `--force`. `migrate --dry-run` of either form reports the rewrite as a safer sequence to run and exits 0 while the real run refuses and exits 2, so the dry-run exit code is not a submit gate for a detach. A desired schema cannot express a detach, so `diff` never produces one" - id: partitioned-parents-in-the-declarative-model area: "partitioned_tables" operation: "Partitioned parents in the **declarative model**" @@ -498,8 +504,8 @@ rows: front_doors: migrate: refused diff: refused - refusal_reason: backend-unavailable - reason_notes: "PostgreSQL has no `DROP VALUE`; this is a type swap + table rewrite โ€” routes to a typed refusal toward copy-and-swap" + refusal_reason: unsupported-statement + reason_notes: "PostgreSQL has no `DROP VALUE`; this is a type swap + table rewrite, planned on copy-and-swap. Today the imperative door refuses `ALTER TYPE ... RENAME VALUE` as `unsupported-statement`, since it takes only `ALTER TABLE` and `CREATE INDEX`, and `DROP VALUE` does not parse" - id: enum-domain-type-creation-and-drop area: "types_and_non_table_objects" operation: "Enum/domain type creation and drop" diff --git a/pkg/capabilities/front_door_integration_test.go b/pkg/capabilities/front_door_integration_test.go new file mode 100644 index 0000000..b74df0c --- /dev/null +++ b/pkg/capabilities/front_door_integration_test.go @@ -0,0 +1,228 @@ +package capabilities_test + +import ( + "fmt" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" + "github.com/block/pg-sprite/pkg/capabilities" + "github.com/block/pg-sprite/pkg/dbconn" + "github.com/block/pg-sprite/pkg/migrate" + "github.com/block/pg-sprite/pkg/statement" + "github.com/block/pg-sprite/pkg/verdict" +) + +// migrateClaim is one row's published promise that the migrate front door +// executes the operation: the schema it needs, and the statements that +// exercise it. %[1]s is the fixture schema, already safely named. +type migrateClaim struct { + setup string + statements []string + // minMajor is the oldest PostgreSQL major the row claims support on; + // zero means every supported major. + minMajor int +} + +// migrateClaims maps every capability row whose imperative `migrate` door is +// published as supported to statements that must run through the real +// migrate.Run (gate, classification, routing, preflight, and sequence +// admission). A row the engine refuses cannot be published as supported: +// consumers read the matrix to decide whether to submit the statement. +var migrateClaims = map[string]migrateClaim{ + "add-column-no-default-or-constant-default": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY + )`, + statements: []string{ + `ALTER TABLE %[1]s.accounts ADD COLUMN nickname text`, + `ALTER TABLE %[1]s.accounts ADD COLUMN tier int NOT NULL DEFAULT 0`, + }, + }, + "drop-column": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + legacy_id int + )`, + statements: []string{`ALTER TABLE %[1]s.accounts DROP COLUMN legacy_id`}, + }, + "alter-column-type-binary-coercible-proven-against-live-column-facts": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + nickname varchar(50) + )`, + statements: []string{`ALTER TABLE %[1]s.accounts ALTER COLUMN nickname TYPE varchar(100)`}, + }, + "set-default-drop-default-drop-not-null": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + nickname text NOT NULL + )`, + statements: []string{ + `ALTER TABLE %[1]s.accounts ALTER COLUMN nickname SET DEFAULT 'anon'`, + `ALTER TABLE %[1]s.accounts ALTER COLUMN nickname DROP DEFAULT`, + `ALTER TABLE %[1]s.accounts ALTER COLUMN nickname DROP NOT NULL`, + }, + }, + "set-not-null": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + nickname text + )`, + statements: []string{`ALTER TABLE %[1]s.accounts ALTER COLUMN nickname SET NOT NULL`}, + }, + "rename-column-rename-table": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + nickname text + )`, + statements: []string{ + `ALTER TABLE %[1]s.accounts RENAME COLUMN nickname TO display_name`, + `ALTER TABLE %[1]s.accounts RENAME TO members`, + }, + }, + "add-primary-key-add-unique-plain-key-columns": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint NOT NULL, + email text NOT NULL + )`, + statements: []string{ + `ALTER TABLE %[1]s.accounts ADD PRIMARY KEY (id)`, + `ALTER TABLE %[1]s.accounts ADD CONSTRAINT accounts_email_key UNIQUE (email)`, + }, + }, + "add-check-add-foreign-key-imperative": { + setup: ` + CREATE TABLE %[1]s.owners ( + id bigint PRIMARY KEY + ); + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + owner_id bigint, + balance int + )`, + statements: []string{ + `ALTER TABLE %[1]s.accounts ADD CONSTRAINT balance_nonnegative CHECK (balance >= 0)`, + `ALTER TABLE %[1]s.accounts ADD CONSTRAINT accounts_owner_fk FOREIGN KEY (owner_id) REFERENCES %[1]s.owners (id)`, + }, + }, + "add-constraint-not-valid-using-index-validate-constraint": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + balance int + )`, + statements: []string{ + `ALTER TABLE %[1]s.accounts ADD CONSTRAINT balance_nonnegative CHECK (balance >= 0) NOT VALID`, + `ALTER TABLE %[1]s.accounts VALIDATE CONSTRAINT balance_nonnegative`, + }, + }, + "add-foreign-key-not-valid-on-a-partitioned-parent": { + setup: ` + CREATE TABLE %[1]s.owners ( + id bigint PRIMARY KEY + ); + CREATE TABLE %[1]s.events ( + id bigint NOT NULL, + owner_id bigint, + PRIMARY KEY (id) + ) PARTITION BY RANGE (id)`, + statements: []string{ + `ALTER TABLE %[1]s.events ADD CONSTRAINT events_owner_fk FOREIGN KEY (owner_id) REFERENCES %[1]s.owners (id) NOT VALID`, + }, + minMajor: 18, + }, + "drop-constraint": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + balance int, + CONSTRAINT balance_nonnegative CHECK (balance >= 0) + )`, + statements: []string{`ALTER TABLE %[1]s.accounts DROP CONSTRAINT balance_nonnegative`}, + }, + "create-unique-index-on-a-plain-table-including-partial-expression-covering-include-gin-gist-brin": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + email text NOT NULL, + deleted_at timestamptz + )`, + statements: []string{ + `CREATE UNIQUE INDEX accounts_email_live_idx ON %[1]s.accounts (lower(email)) WHERE deleted_at IS NULL`, + }, + }, +} + +// declarativeOnlyClaims are the rows whose migrate door is the desired-state +// path (migrate --desired), not an imperative statement; their execution is +// covered by the desired-state integration tests in pkg/migrate. +var declarativeOnlyClaims = []string{ + "plain-tables-their-indexes", + "greenfield-create-table-apply-the-table-does-not-exist-yet-a-fresh-database-or-a-new-table-in-a-live-one", + "declarative-row-security-library", + "declarative-not-valid-constraint", +} + +// Every capability row that publishes `front_doors.migrate: supported` must +// have its representative statements actually executed by migrate.Run. A +// refusal at the gate, the router, preflight, or sequence admission means the +// matrix promised an operation the shipped migrate command rejects. +func TestMigrateSupportedRowsExecute(t *testing.T) { + rows, err := capabilities.Rows() + require.NoError(t, err) + + var supported []string + for _, row := range rows { + if row.FrontDoors.Migrate == capabilities.DoorSupported { + supported = append(supported, row.ID) + } + } + for _, id := range supported { + _, imperative := migrateClaims[id] + assert.True(t, imperative || slices.Contains(declarativeOnlyClaims, id), + "row %s publishes migrate: supported but has no representative statement in migrateClaims", id) + } + + url := testutil.StartPostgres(t) + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: url}) + require.NoError(t, err) + defer pool.Close() + major, err := dbconn.ServerMajor(t.Context(), pool) + require.NoError(t, err) + + for _, id := range supported { + claim, ok := migrateClaims[id] + if !ok { + continue + } + t.Run(id, func(t *testing.T) { + if major < claim.minMajor { + t.Skipf("row claims support from PostgreSQL %d; server is %d", claim.minMajor, major) + } + schema := testutil.NewSchema(t, pool) + _, err := pool.Exec(t.Context(), fmt.Sprintf(claim.setup, schema)) + require.NoError(t, err) + + for _, sql := range claim.statements { + sql = fmt.Sprintf(sql, schema) + st, err := statement.ParseOne(sql) + require.NoError(t, err) + v, err := migrate.Run(t.Context(), pool, st, migrate.DefaultOptions()) + require.NoError(t, err, sql) + assert.Equal(t, verdict.OutcomeExecuted, v.Outcome, + "%s: the matrix publishes migrate as supported but it answered %s (%s): %s", sql, v.Outcome, v.Reason, v.Detail) + } + }) + } +} diff --git a/pkg/capabilities/front_door_refusal_integration_test.go b/pkg/capabilities/front_door_refusal_integration_test.go new file mode 100644 index 0000000..ef349b0 --- /dev/null +++ b/pkg/capabilities/front_door_refusal_integration_test.go @@ -0,0 +1,149 @@ +package capabilities_test + +import ( + "fmt" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" + "github.com/block/pg-sprite/pkg/capabilities" + "github.com/block/pg-sprite/pkg/dbconn" + "github.com/block/pg-sprite/pkg/migrate" + "github.com/block/pg-sprite/pkg/statement" + "github.com/block/pg-sprite/pkg/verdict" +) + +// migrateRefusals maps rows published as `migrate: refused` to statements the +// migrate front door must refuse with the row's refusal_reason. Consumers +// filter on the published token, so it must be the one migrate emits. +var migrateRefusals = map[string]migrateClaim{ + // The attach scans the partition to prove its bound, and the engine + // does not yet construct the CHECK that would let it skip the scan. + "attach-partition": { + setup: ` + CREATE TABLE %[1]s.events ( + id bigint NOT NULL, + PRIMARY KEY (id) + ) PARTITION BY RANGE (id); + CREATE TABLE %[1]s.events_low ( + id bigint NOT NULL, + PRIMARY KEY (id) + )`, + statements: []string{ + `ALTER TABLE %[1]s.events ATTACH PARTITION %[1]s.events_low FOR VALUES FROM (0) TO (1000)`, + }, + }, + // The plain form is rewritten to CONCURRENTLY, and the executor does not + // drive a concurrent detach; both forms refuse the same way. + "detach-partition-concurrently": { + setup: ` + CREATE TABLE %[1]s.events ( + id bigint NOT NULL, + PRIMARY KEY (id) + ) PARTITION BY RANGE (id); + CREATE TABLE %[1]s.events_low PARTITION OF %[1]s.events FOR VALUES FROM (0) TO (1000)`, + statements: []string{ + `ALTER TABLE %[1]s.events DETACH PARTITION %[1]s.events_low`, + `ALTER TABLE %[1]s.events DETACH PARTITION %[1]s.events_low CONCURRENTLY`, + }, + }, + // The imperative door does not drive index maintenance: the plain form + // names its concurrent idiom, and the concurrent form is run directly. + "drop-index": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + email text NOT NULL + ); + CREATE INDEX accounts_email_idx ON %[1]s.accounts (email)`, + statements: []string{ + `DROP INDEX %[1]s.accounts_email_idx`, + `DROP INDEX CONCURRENTLY %[1]s.accounts_email_idx`, + }, + }, + "reindex": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY, + email text NOT NULL + ); + CREATE INDEX accounts_email_idx ON %[1]s.accounts (email)`, + statements: []string{ + `REINDEX INDEX %[1]s.accounts_email_idx`, + `REINDEX INDEX CONCURRENTLY %[1]s.accounts_email_idx`, + }, + }, + // The imperative door takes only ALTER TABLE and CREATE INDEX, so an + // ALTER TYPE is refused at the gate. + "enum-value-rename-removal": { + setup: `CREATE TYPE %[1]s.mood AS ENUM ('happy', 'sad')`, + statements: []string{ + `ALTER TYPE %[1]s.mood RENAME VALUE 'sad' TO 'blue'`, + }, + }, + // The inline constraint would build its index under the ADD COLUMN's + // ACCESS EXCLUSIVE lock; the safe form adds the column first. + "add-column-with-inline-unique-primary-key-references-check": { + setup: ` + CREATE TABLE %[1]s.accounts ( + id bigint PRIMARY KEY + )`, + statements: []string{ + `ALTER TABLE %[1]s.accounts ADD COLUMN code int UNIQUE`, + }, + }, +} + +// A row published as `migrate: refused` must be refused by migrate.Run with +// exactly the published refusal_reason. +func TestMigrateRefusedRowsRefuse(t *testing.T) { + rows, err := capabilities.Rows() + require.NoError(t, err) + byID := map[string]capabilities.Row{} + for _, row := range rows { + byID[row.ID] = row + } + + url := testutil.StartPostgres(t) + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: url}) + require.NoError(t, err) + defer pool.Close() + + for id, claim := range migrateRefusals { + t.Run(id, func(t *testing.T) { + row, ok := byID[id] + require.True(t, ok, "row %s is not in the registry", id) + require.Equal(t, capabilities.DoorRefused, row.FrontDoors.Migrate) + schema := testutil.NewSchema(t, pool) + _, err := pool.Exec(t.Context(), fmt.Sprintf(claim.setup, schema)) + require.NoError(t, err) + + for _, sql := range claim.statements { + sql = fmt.Sprintf(sql, schema) + st, err := statement.ParseOne(sql) + require.NoError(t, err) + v, err := migrate.Run(t.Context(), pool, st, migrate.DefaultOptions()) + require.NoError(t, err, sql) + assert.Equal(t, verdict.OutcomeRefused, v.Outcome, sql) + assert.Equal(t, row.RefusalReason, v.Reason, + "%s: the matrix publishes a different reason than migrate emits", sql) + } + }) + } +} + +// A representative statement is only evidence for a row the matrix publishes +// as supported. +func TestMigrateClaimsBelongToSupportedRows(t *testing.T) { + rows, err := capabilities.Rows() + require.NoError(t, err) + for id := range migrateClaims { + i := slices.IndexFunc(rows, func(r capabilities.Row) bool { return r.ID == id }) + require.GreaterOrEqual(t, i, 0, "row %s is not in the registry", id) + assert.Equal(t, capabilities.DoorSupported, rows[i].FrontDoors.Migrate, + "migrateClaims has statements for row %s, which is not published as migrate: supported", id) + } +}