From 51f5765407e06ba0355f827430564be184e9da06 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Wed, 28 Oct 2026 16:13:17 +1100 Subject: [PATCH] schemachange: refuse a new column the copy cannot fill at the shadow build A column added beside the gated changes, or the key dropped or replaced under a new name, built and committed a shadow that failed at the first chunk and was reported as a data fault. Refuse it in the build's transaction instead, as shadow-statement-new-column. --- README.md | 5 +- docs/capabilities.md | 2 +- docs/invariants.md | 24 +++- docs/limitations.md | 3 +- docs/refusal-classes.md | 11 +- pkg/capabilities/capabilities.yaml | 2 +- pkg/schemachange/new_column.go | 45 +++++++ .../new_column_integration_test.go | 125 ++++++++++++++++++ pkg/schemachange/new_column_test.go | 108 +++++++++++++++ pkg/schemachange/refusal.go | 17 ++- pkg/schemachange/refusal_test.go | 2 + pkg/schemachange/shadow.go | 7 + 12 files changed, 334 insertions(+), 17 deletions(-) create mode 100644 pkg/schemachange/new_column.go create mode 100644 pkg/schemachange/new_column_integration_test.go create mode 100644 pkg/schemachange/new_column_test.go diff --git a/README.md b/README.md index 2acaffa5..12a18d91 100644 --- a/README.md +++ b/README.md @@ -70,8 +70,9 @@ refusal — never a silently wrong or incomplete result: through to a blocking rewrite, and the route's preflight refuses table shapes and environments it cannot run against. The shadow build also refuses a statement the copy cannot carry: a `USING` expression it cannot reproduce, - a dropped-and-re-added column it cannot fill, or a new identity or `serial` - column whose sequence the swap cannot carry. + a dropped-and-re-added or new column it cannot fill, a key it would not + keep, or a new identity or `serial` column whose sequence the swap cannot + carry. - **Row security** is included in exports when present, with reviewable before/after differences. The [atomic executor](docs/atomic-row-security.md) applies RLS-only changes to existing tables through `migrate --desired` or the Go API. Only the Go API supports binding execution to diff --git a/docs/capabilities.md b/docs/capabilities.md index b4d2bf8b..00ca6717 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -208,7 +208,7 @@ or the only online form is a direct statement the operator runs). | --- | --- | --- | --- | --- | | `ADD COLUMN` (no default, a constant default, or a non-volatile default such as `now()`) | ✅ | native, as-is | Yes | Metadata-only / fast default (PG 11+), proven by adding the column to an empty temporary table first; executes instantly under bounded locks | | `ADD COLUMN` with volatile default (`gen_random_uuid()`, `clock_timestamp()`, identity, `serial`, …) | 🟡 | copy-and-swap | Yes | Table rewrite; routes to copy-and-swap, which does not carry a volatile default yet (the shadow fill would have to evaluate the default per row and the stream would have to agree). An identity or `serial` column is further refused at the shadow build itself with `shadow-statement-new-sequence`, as a sequence the swap cannot carry to the live table — the engine's guard for library callers that reach the build without this route check — see [limitations.md](limitations.md) | -| `ADD COLUMN` of a domain type with a `CHECK` or `NOT NULL`, with or without a default | 🟡 | copy-and-swap | Yes | Table rewrite to check every row against the domain; routes to copy-and-swap, which does not carry an added column yet (the shadow fill and the stream would each fill it from the default) | +| `ADD COLUMN` of a domain type with a `CHECK` or `NOT NULL`, with or without a default | 🟡 | copy-and-swap | Yes | Table rewrite to check every row against the domain; routes to copy-and-swap, which does not carry an added column yet (the shadow fill and the stream would each fill it from the default). A library caller that reaches the shadow build with a domain that refuses NULL and has no default is refused there with shadow-statement-new-column | | `ADD COLUMN ... GENERATED ... STORED` | 🟡 | copy-and-swap | Yes | Table rewrite; routes to copy-and-swap, which does not carry stored generated columns yet — the copy must **recompute, never copy,** them on the shadow table | | `ALTER COLUMN ... SET EXPRESSION` (changed stored generated expression) | 🟡 | copy-and-swap | Yes | Planned but not executed. `diff` shows a changed stored generated expression as one `SET EXPRESSION` statement of kind `set-generated-expression`, refused with `unsupported-statement`, rather than a drop and re-add that would also drop the column's indexes. It recomputes every row under `ACCESS EXCLUSIVE`, and the statement needs PostgreSQL 17 or later | | `ADD COLUMN` with inline `UNIQUE`/`PRIMARY KEY`/`REFERENCES`/`CHECK` | 🟡 | native, planned flow | Yes | 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 | diff --git a/docs/invariants.md b/docs/invariants.md index 6efcc8e8..56bb8ed2 100644 --- a/docs/invariants.md +++ b/docs/invariants.md @@ -1279,14 +1279,21 @@ field for field against the checkpointed proof. A re-added column the copy canno its default — the primary key, the replacement of a source identity column, or a column that refuses NULL, by its own `NOT NULL` or a domain's, with no default of its own or on its type — is refused inside the build's transaction with `shadow-statement-readd`, so no shadow is -committed for a statement whose copy could not run. A column re-added as an identity or -`serial` is fillable by the server, so it passes this check and is refused by ST-6's +committed for a statement whose copy could not run. The same holds for every column the copy +list leaves out, not only a re-add: a new column that refuses NULL with nothing the server +fills it from, and the source's primary key absent from the kept columns — dropped outright or +replaced by a key under a new name, which no chunk range, watermark, or streamed change could +be expressed in on the shadow — are refused in the same transaction with +`shadow-statement-new-column`. A column re-added or added as an identity or `serial` is +fillable by the server, so it passes both checks and is refused by ST-6's `shadow-statement-new-sequence` instead. *Enforced:* `pkg/schemachange` (`readColumnPositions` reads both relations' live attribute numbers inside the build's and the inspection's transaction; `sameColumns` keeps a column only when its live position equals its shadow attribute number; `copyColumns` takes only those; `readDomainFills` reads what each shadow column's domain contributes; `refuseUnfillableReadds` -refuses the three re-adds before the build commits). Tests: +refuses the three re-adds before the build commits; `refuseUnfillableNewColumns` then refuses a +shadow without the primary key among its kept columns, and any other left-out column that +refuses NULL with nothing to fill it). Tests: `TestSameColumnsComparesLivePositionToShadowNumber`, `TestRefuseUnfillableReaddsAdmitsAColumnTheServerFills`, `TestRefuseUnfillableReaddsRefusesThePrimaryKey`, @@ -1295,6 +1302,17 @@ refuses the three re-adds before the build commits). Tests: `TestRefuseUnfillableReaddsIgnoresColumnsTheStatementKeptOrDropped`, `TestRefuseUnfillableReaddsAdmitsANotNullColumnWhoseDomainDefaultsIt`, `TestRefuseUnfillableReaddsRefusesAColumnWhoseDomainRefusesNull`, +`TestRefuseUnfillableNewColumnsAdmitsAColumnTheServerFills`, +`TestRefuseUnfillableNewColumnsRefusesNotNullWithoutADefault`, +`TestRefuseUnfillableNewColumnsRefusesAShadowWithoutThePrimaryKey`, +`TestRefuseUnfillableNewColumnsIgnoresColumnsTheStatementKeptOrDropped`, +`TestRefuseUnfillableNewColumnsAdmitsANotNullColumnWhoseDomainDefaultsIt`, +`TestRefuseUnfillableNewColumnsRefusesAColumnWhoseDomainRefusesNull`, +`TestBuildShadowRefusesANewNotNullColumnWithNoDefault`, +`TestBuildShadowRefusesANewColumnWhoseDomainRefusesNull`, +`TestBuildShadowRefusesAStatementThatReplacesThePrimaryKey`, +`TestBuildShadowAdmitsANewColumnTheServerFills`, +`TestRunnerRefusesAnUnfillableNewColumnBeforeAnyRowOrFence`, `TestBuildShadowLeavesAReaddedColumnOutOfTheCopyWhenTheSourceHasADroppedColumn`, `TestBuildShadowCopiesEveryColumnTheChangeKeeps`, `TestBuildShadowRefusesAReaddedNotNullColumnWithNoDefault`, diff --git a/docs/limitations.md b/docs/limitations.md index 897c6f73..b04e0018 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -28,7 +28,8 @@ not escape hatches: | `ALTER COLUMN TYPE ... USING ` the copy cannot reproduce | The copy-and-swap route (above) runs the retype, but the shadow builder admits a `USING` expression only when it can evaluate it again, identically, in the chunk copy, the verifier's recopy, and every change the catch-up flushes. Three rules, each refused as `shadow-statement-conversion` before any shadow is committed: the expression must be **immutable** by the server's own classification (no `now()`, `random()`, `current_setting()`, session-dependent casts such as `text` to `timestamptz`, or functions declared `STABLE` or `VOLATILE`; checked by trying it as a stored generated column's expression on the empty shadow inside the build's transaction, then rolling that attempt back), it must **name only the column it retypes**, bare and unqualified (no neighbouring column, no `t.col`, no whole-row `t.*`), and it must not retype the **primary key**, whose values every chunk range, watermark, and resume clear are expressed in. The expression is also refused when the deparser cannot prove it rendered the text faithfully. Rewrite the expression in terms of the retyped column alone and of `IMMUTABLE` functions, or retype without `USING` so the server's assignment cast converts the value; the refusal names the column, the expression, and that rewrite. The spellings most often refused, and what to write instead, are in [`USING` expressions the copy reproduces](#using-expressions-the-copy-reproduces) below. | | A schema change to the source table while a copy-and-swap runs | Unsupported: logical decoding does not stream DDL, and the copy does not lock it out — a chunk holds `ACCESS SHARE` on the source only for its own transaction, so `ALTER TABLE` on the source completes between chunks. The copy selects the source by column name, so what happens depends on whether every copied name still resolves. A copied column renamed or dropped stops the copy at its next chunk with the server's `undefined_column` error (SQLSTATE `42703`), which is not a typed refusal; that chunk stays in flight and nothing from it reaches the shadow. A change that leaves every copied name resolving to a column of a compatible type — two columns swapping names, a column renamed and re-added under the old name, a widened type — is not detected by the copy, which completes, and after a swap or a re-add the rows copied since hold another column's values or the new column's default. Once the replication stream has decoded a change to the table, a later relation description that differs stops it with `ErrSourceShapeChanged`. None of these reaches the swap: the verifier compares source and shadow rows by column name, and the cutover gate re-fingerprints the source and refuses any changed column as `cutover-schema-drift` ([refusal-classes.md](refusal-classes.md#shadow-operation-refusals-keyed-on-refusalcause)), after which the shadow is dropped and the change re-planned. | | A concurrent index build whose name is occupied by an invalid index, or whose table carries quarantined debris | The build refuses before running and never drops the occupant: an invalid entry is either abandoned debris or someone's build still in progress, and a drop by name cannot tell them apart. The refusal is typed by proof (`invalid-index-abandoned`, `invalid-index-build-in-flight`, `invalid-index-other-table`, `invalid-index-not-droppable`, `invalid-index-builder-unobservable`), and a table carrying an entry a previous recovery quarantined (`pgsprite_abandoned_`) but did not drop is refused the same way, so debris never goes quiet. The proven removal — `executor.RebuildAbandonedIndex`, or `executor.DropAbandonedIndex` to remove the leftover without running the build; both library-only today, no CLI verb — takes a bounded `SHARE UPDATE EXCLUSIVE` lock on the table (which any running concurrent index command holds), re-verifies the entry by OID, renames it to an identity-derived name and drops that, all within one shared budget; it refuses a visible in-flight build, another table's entry, or an entry the server will not drop concurrently (a partitioned table's index, an index partition, a constraint's index), and reports a lock budget rather than wait behind a build it cannot see. From the CLI, the [runbook](invalid-index-recovery.md) applies. | -| A copy-and-swap statement that drops a column and adds one of the same name | Like any drop, it runs only under `--allow-destructive`; without it the statement is refused with `destructive-change` and the `alter-column-type-in-place` guidance before the shadow build starts. Under the acknowledgement, the re-added column is a new column: every row holds its default (or NULL), as under a direct `ALTER TABLE`, and the dropped column's values are not copied into it ([copy-and-swap D1](copy-and-swap-design.md#d1--no-durable-scratch-database)). Three such re-adds have no default to take or are the key the copy is driven by, and the shadow build refuses them with `shadow-statement-readd` inside its own transaction, before any shadow, fence, or checkpoint row exists: re-adding the primary key column, re-adding a source identity column, and re-adding a column that refuses NULL (by its own `NOT NULL` or its domain's) with no default of its own or on its type. The remedy depends on which: a primary key is refused whatever the new column declares, so a default does not help — change the key in place with `ALTER COLUMN TYPE`; a source identity column is refused whatever the new column declares too — drop it in one schema change and add the new column in the next; any other column runs once the re-added column has a default (its own, or its domain's). A nullable, defaulted, or generated re-add builds and runs. A re-add declared as an identity or `serial` is refused in the same transaction with `shadow-statement-new-sequence` (next row). The refusal covers only these named re-add shapes: a new column the server cannot fill in other ways — a `CHECK` that rejects the column's default or NULL, a `NOT NULL` column added without a drop beside a retype, or the key replaced under a new name — still builds and stops at the first chunk or the copier's shape check, failing closed; a first-chunk stop is the server's class `23` error, a data fault, so the run abandons itself and the verdict is `failed` with `code: data-fault`. A run checkpointed by an earlier build for a drop-and-re-add statement no longer resumes (`resume-row-mismatch`, because the copy list it recorded names the re-added column); abandon it and run again rather than retrying the resume. | +| A copy-and-swap statement that drops a column and adds one of the same name | Like any drop, it runs only under `--allow-destructive`; without it the statement is refused with `destructive-change` and the `alter-column-type-in-place` guidance before the shadow build starts. Under the acknowledgement, the re-added column is a new column: every row holds its default (or NULL), as under a direct `ALTER TABLE`, and the dropped column's values are not copied into it ([copy-and-swap D1](copy-and-swap-design.md#d1--no-durable-scratch-database)). Three such re-adds have no default to take or are the key the copy is driven by, and the shadow build refuses them with `shadow-statement-readd` inside its own transaction, before any shadow, fence, or checkpoint row exists: re-adding the primary key column, re-adding a source identity column, and re-adding a column that refuses NULL (by its own `NOT NULL` or its domain's) with no default of its own or on its type. The remedy depends on which: a primary key is refused whatever the new column declares, so a default does not help — change the key in place with `ALTER COLUMN TYPE`; a source identity column is refused whatever the new column declares too — drop it in one schema change and add the new column in the next; any other column runs once the re-added column has a default (its own, or its domain's). A nullable, defaulted, or generated re-add builds and runs. A `NOT NULL` column added beside the other changes rather than re-added, or the key dropped or replaced under a new name, is refused by the next check, `shadow-statement-new-column` (next row). A re-add declared as an identity or `serial` is refused in the same transaction with `shadow-statement-new-sequence` (row after next). The refusal covers only what the catalog states: a new column the server cannot fill in another way — a `CHECK` that rejects the column's default or NULL — still builds and stops at the first chunk, failing closed; that stop is the server's class `23` error, a data fault, so the run abandons itself and the verdict is `failed` with `code: data-fault`. A run checkpointed by an earlier build for a drop-and-re-add statement no longer resumes (`resume-row-mismatch`, because the copy list it recorded names the re-added column); abandon it and run again rather than retrying the resume. | +| A copy-and-swap statement that adds a column the copy cannot fill, or leaves the table without its key | The copy list names only the source columns the shadow kept as the same column ([ST-11](invariants.md#st-11--the-shadow-takes-a-columns-values-only-from-the-same-source-column)); every other shadow column is a new column that takes its own default (or NULL) on every copied row, as under a direct `ALTER TABLE`. Two shapes have nothing to take, and the shadow build refuses them with `shadow-statement-new-column` inside its own transaction, after the statement has run on the empty shadow and both relations' column positions have been read, so no shadow, fence, or checkpoint row exists: a new column that refuses NULL (by its own `NOT NULL` or its domain's, including a domain its type is built on) with no default of its own or on its type, no generation expression, and no identity — `ALTER COLUMN qty TYPE bigint, ADD COLUMN region text NOT NULL` — and the source's primary key missing from the shadow's kept columns, dropped outright or replaced by a key under a new name — `DROP COLUMN id, ADD COLUMN new_id bigint PRIMARY KEY, ALTER COLUMN qty TYPE bigint`. Without the refusal both would build and commit a shadow, then stop at the first chunk: the first with the server's `not_null_violation`, a data fault that abandons the run and reports `code: data-fault` for a statement no data could have satisfied; the second on a copy keyed on a column the shadow does not have. From the CLI these shapes reach the build only beside an operation that routes to copy-and-swap, such as the retype above: on its own, `ADD COLUMN … NOT NULL` without a default is routed native and the server rejects it on a non-empty table, `ADD COLUMN` of a `NOT NULL` domain is `backend-unavailable`, and a dropped key is native. Beside a retype, every natively-routed operation rides along into the gated statement, so the build-time refusal is what stands between the operator and the first-chunk fault; it is also the library path's guard (`schemachange.Runner`, `BuildShadow`). A missing key is refused whatever the new key declares — change the key in place with `ALTER COLUMN TYPE`. For a new column, give it a default (its own, or its domain's), or add it in a later schema change the server applies in place; a nullable, defaulted, or generated new column builds and runs. A new column declared as an identity or `serial` is fillable and is refused by the next check, `shadow-statement-new-sequence` (next row). | | A copy-and-swap statement that gives the table a new identity or `serial` column | The swap carries only the source's sequences to the live table — each re-owned by the shadow column that kept its source column's name — so a sequence the gated statement mints on the shadow (`ADD COLUMN … GENERATED … AS IDENTITY`, `ADD COLUMN … serial`, `ALTER COLUMN … ADD GENERATED … AS IDENTITY` on a kept column, or a dropped column re-added as one) has no source counterpart to carry. The shadow build refuses such a statement with `shadow-statement-new-sequence` inside its own transaction, after the statement has run on the empty shadow and the shadow's columns and sequence-ownership edges have been read, so no shadow, fence, or checkpoint row exists; the source's own serial and identity columns are not new sequences and pass. From the CLI these statements are routed before the build reaches them — a new identity or `serial` column is a volatile-default `ADD COLUMN`, which the copy-and-swap backend does not carry yet (`backend-unavailable`), and `ADD GENERATED` on an existing column is `unsupported-operation` — so the build-time refusal is the library path's guard (`schemachange.Runner`, `BuildShadow`) and the engine's own defence should a route later admit them. Declare the column plain in this schema change and add the identity, or attach the sequence, in a later one that the server applies in place. | ## `USING` expressions the copy reproduces diff --git a/docs/refusal-classes.md b/docs/refusal-classes.md index e67b8349..7e5164c9 100644 --- a/docs/refusal-classes.md +++ b/docs/refusal-classes.md @@ -165,11 +165,11 @@ reads it through wrapping. Every one of these refusals is fail-closed under `ErrInvariantViolation` ([SAFETY.md](../SAFETY.md#rules-inside-the-core)): the sentinel is the mechanism that stops the operation, and the cause is what tells the importer which way to react. Only the causes that mean the caller handed the engine an incoherent input, or that the -engine's own write did not take, are `invariant-violation`; three, `shadow-statement-conversion`, -`shadow-statement-readd`, and `shadow-statement-new-sequence`, are `capability-boundary`, since -each describes a statement the engine declines to carry rather than a fault; the rest describe a -lock, a proof, or a relation whose state moved under the operation, and the importer retries, -re-plans, or cleans up. +engine's own write did not take, are `invariant-violation`; four, `shadow-statement-conversion`, +`shadow-statement-readd`, `shadow-statement-new-column`, and `shadow-statement-new-sequence`, are +`capability-boundary`, since each describes a statement the engine declines to carry rather than +a fault; the rest describe a lock, a proof, or a relation whose state moved under the operation, +and the importer retries, re-plans, or cleans up. `RefusalCause.Class()` answers the `class` column below as a `refusal.Class` (`pkg/refusal`, the core's declaration of the classes the cause tables in this document use), so an importer triages on the class without reading the cause's text, and a periphery adapter converts it to @@ -196,6 +196,7 @@ is `RefusalCause.Class()` converted by token (`pkg/migrate/copy_swap_refusal.go` | `shadow-grants-differ` | The shadow's grants still differ from the source's after the build synchronised them ([ST-5](invariants.md#st-5--the-swap-is-gated-on-a-fidelity-checklist-not-just-the-checksum)) | `invariant-violation` | The build wrote the grants under the table lock and read back something else; that is the engine's write, not the environment. | | `shadow-identity-handoff` | A source identity column the change kept on the shadow does not carry `DEFAULT nextval()` there, or the shadow declares an identity of its own ([ST-5](invariants.md#st-5--the-swap-is-gated-on-a-fidelity-checklist-not-just-the-checksum)) | `environmental` | The change itself, or a later edit to the shadow, replaced the handoff; change the statement or drop the shadow and rebuild. | | `shadow-statement-readd` | The gated statement drops a column and adds one under the same name that the copy cannot fill: the primary key, whose values the copy's chunk ranges and watermark are expressed in; a source identity column, whose sequence is handed only to the same column kept on the shadow; or a column that refuses NULL — by its own `NOT NULL` or a domain's, including a domain its type is built on — with no default of its own, no default on its type, no generation expression, and no identity, so the first chunk would have no value to give it ([ST-11](invariants.md#st-11--the-shadow-takes-a-columns-values-only-from-the-same-source-column)) | `capability-boundary` | A re-added column is a new column that takes its default, never the dropped column's values, and these three have no default to take or are the key the copy is driven by. The build refuses inside its own transaction, after the statement has run on the empty shadow and its column positions have been read, so nothing is committed and no checkpoint row is written. Re-plan the statement rather than retry it; the remedy depends on the column. A primary key is refused whatever the new column declares — a default does not lift it — so change the key in place with `ALTER COLUMN TYPE`. A source identity column is refused whatever the new column declares too: drop it in one schema change and add the new column in the next. For any other column, give the re-added column a default (its own, or its domain's). A nullable, defaulted, or generated re-add builds and runs; a re-add declared as an identity or `serial` is refused by the next check, `shadow-statement-new-sequence`, as any new identity or sequence-owned column through copy-and-swap is. The message names the column and the remedy; the cause carries no column. | +| `shadow-statement-new-column` | The gated statement leaves the shadow with a column the copy cannot fill, or without the column the copy is keyed on: a new column — added beside the other changes, not re-added under a dropped column's name — that refuses NULL, by its own `NOT NULL` or a domain's, including a domain its type is built on, with no default of its own, no default on its type, no generation expression, and no identity, so the first chunk would have no value to give it; or the source's primary key missing from the shadow's kept columns — dropped outright, or replaced by a key under a new name — so no chunk range, watermark, or streamed change can name it there ([ST-11](invariants.md#st-11--the-shadow-takes-a-columns-values-only-from-the-same-source-column)) | `capability-boundary` | The copy list names only the source columns the shadow kept as the same column; every other shadow column is a new column that takes its default on every copied row, and these two shapes have no default to take or are the key the copy is driven by. The build refuses inside its own transaction, after the statement has run on the empty shadow and both relations' column positions have been read, so nothing is committed and no checkpoint row is written — the alternative was the first chunk's `not_null_violation`, a data fault that abandons a committed shadow. Re-plan the statement rather than retry it. A missing primary key is refused whatever the new key declares — change the key in place with `ALTER COLUMN TYPE`. For a new column, give it a default (its own, or its domain's), or add it in a later schema change the server applies in place. A nullable, defaulted, or generated new column builds and runs; a new column declared as an identity or `serial` is fillable and is refused by the next check, `shadow-statement-new-sequence`. A dropped column re-added under its own name is refused before this check as `shadow-statement-readd`. The message names the column and the remedy; the cause carries no column. | | `shadow-statement-new-sequence` | The gated statement gives the shadow an identity column, or a column owning a sequence (`serial`), that the source does not have: a new column declared so, a kept column given `ADD GENERATED … AS IDENTITY`, or a dropped column re-added as one ([ST-6](invariants.md#st-6--preflight-before-the-first-write)) | `capability-boundary` | The swap carries only the source's sequences to the live table, re-owning each by the shadow column that kept its source column's name; a sequence the statement minted on the shadow has no source counterpart to carry, so the swap's catalog re-read would stop on it after the whole copy. The build refuses inside its own transaction instead, after the statement has run on the empty shadow and the shadow's columns and `pg_depend` ownership edges have been read, so nothing is committed and no checkpoint row is written. The source's own serial and identity columns do not trip the check: the shadow draws them from the source's sequences without owning them until the swap. Declare the column plain in this schema change and add the identity, or attach the sequence, in a later one that the server applies in place. The message names the column and the remedy; the cause carries no column. | | `cutover-unverified` | The cutover gate was handed an empty built-shadow or verified-shadow proof, two proofs naming different relations or different relation OIDs (a shadow rebuilt under the same name after the checksum ran), or a verified watermark that does not cover the whole key space ([CO-1](invariants.md#co-1--the-checksum-gate-is-non-skippable)) | `invariant-violation` | Only the copier and verifier mint those proofs for the same table; a caller that reaches the gate without them skipped the checksum, which no retry corrects. | | `cutover-undrained` | The cutover was handed a verified-shadow proof a live pass minted — the shadow equal to the source as of a position the stream had reached, with every change since still in the stream — and no drain to apply those changes under the swap lock ([CO-1](invariants.md#co-1--the-checksum-gate-is-non-skippable)) | `invariant-violation` | Pass a `DrainFunc` that applies the stream through the final position under the swap lock, or stop the stream and re-verify with a quiesced `Check`; the proof is sound, the hand-off to the swap is not. | diff --git a/pkg/capabilities/capabilities.yaml b/pkg/capabilities/capabilities.yaml index c069bdd6..c3f6a6e3 100644 --- a/pkg/capabilities/capabilities.yaml +++ b/pkg/capabilities/capabilities.yaml @@ -33,7 +33,7 @@ rows: migrate: refused diff: refused refusal_reason: backend-unavailable - reason_notes: "Table rewrite to check every row against the domain; routes to copy-and-swap, which does not carry an added column yet (the shadow fill and the stream would each fill it from the default)" + reason_notes: "Table rewrite to check every row against the domain; routes to copy-and-swap, which does not carry an added column yet (the shadow fill and the stream would each fill it from the default). A library caller that reaches the shadow build with a domain that refuses NULL and has no default is refused there with shadow-statement-new-column" - id: add-column-generated-stored area: "column_changes" operation: "`ADD COLUMN ... GENERATED ... STORED`" diff --git a/pkg/schemachange/new_column.go b/pkg/schemachange/new_column.go new file mode 100644 index 00000000..fb8e3110 --- /dev/null +++ b/pkg/schemachange/new_column.go @@ -0,0 +1,45 @@ +package schemachange + +import ( + "github.com/block/pg-sprite/pkg/schemadiff" +) + +// refuseUnfillableNewColumns refuses a gated statement that leaves the +// shadow with a column the copy cannot fill, or without the column the +// copy is keyed on. The copy list names only the source columns the shadow +// kept as the same column; every other shadow column is a new column the +// copy leaves out, and every row the copy writes takes that column's own +// default, or its domain's, as under a direct ALTER TABLE. Two shapes have +// nowhere to go, and are refused here, before the build commits: +// +// - the primary key is not among the kept columns — dropped outright, or +// replaced by a key under a new name — so every chunk range, watermark, +// and streamed change names a column the shadow does not have; +// - a new column that refuses NULL, by its own NOT NULL or its domain's, +// with nothing the server fills it from, which the first copied row +// would violate. +// +// A new column the copy can leave to the server — nullable, with a default +// of its own or its domain's, or generated — is not refused here. A column +// dropped and added again under its own name is a new column too, and is +// refused before this check as the re-add it is; a new identity or serial +// column is fillable by the server and is refused after it as a sequence +// the swap cannot carry. Both position lists are in attribute order, as +// readColumnPositions returns them; pk is the source's primary key column +// and domains is the shadow's readDomainFills. +func refuseUnfillableNewColumns(schema, table, pk string, sourcePositions, shadowPositions []columnPosition, shadow schemadiff.Model, domains map[string]domainFill) error { + same := sameColumns(sourcePositions, shadowPositions) + // INV: ST-11 + if !same[pk] { + return refuse(CauseStatementNewColumn, nil, "the statement for %s.%s leaves the table without its primary key %s, which the copy is keyed on; a key under another name does not lift this, so change the key in place with ALTER COLUMN TYPE instead", schema, table, pk) + } + for _, column := range shadow.Columns { + if same[column.Name] { + continue + } + if requiresAValueTheCopyCannotGive(column, domains[column.Name]) { + return refuse(CauseStatementNewColumn, nil, "the statement for %s.%s adds column %s, which refuses NULL and has no default of its own or from its type, so the copy has no value to give it; give the new column a default, or add it in a later schema change the server applies in place", schema, table, column.Name) + } + } + return nil +} diff --git a/pkg/schemachange/new_column_integration_test.go b/pkg/schemachange/new_column_integration_test.go new file mode 100644 index 00000000..87877a18 --- /dev/null +++ b/pkg/schemachange/new_column_integration_test.go @@ -0,0 +1,125 @@ +package schemachange_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/checkpoint" + "github.com/block/pg-sprite/pkg/schemachange" +) + +// Adding a NOT NULL column with no default beside a retype leaves the copy +// nothing to put in it: the column is not in the copy list, so every copied +// row would take its default, and it has none. The server accepts the +// statement on the empty shadow; the build refuses it inside its own +// transaction rather than committing a shadow whose first chunk would fail. +func TestBuildShadowRefusesANewNotNullColumnWithNoDefault(t *testing.T) { + f := newShadowFixture(t) + f.createOrders(t) + + _, err := f.build(t, "orders", ` + ALTER TABLE %s.orders + ADD COLUMN region text NOT NULL, + ALTER COLUMN qty TYPE bigint`) + + assert.ErrorIs(t, err, schemachange.ErrInvariantViolation) + assert.Equal(t, schemachange.CauseStatementNewColumn, schemachange.RefusalCauseOf(err)) + assert.False(t, f.relationExists(t, schemachange.ShadowName(f.schema, "orders")), "a refused build creates nothing") +} + +// A new column of a domain that refuses NULL, with no default on the column +// or the domain, is refused for the same reason: the column is declared +// nullable, but the server enforces the domain's NOT NULL on every row the +// copy writes. +func TestBuildShadowRefusesANewColumnWhoseDomainRefusesNull(t *testing.T) { + f := newShadowFixture(t) + f.createOrders(t) + f.exec(t, ` + CREATE DOMAIN %s.nonnull_int AS integer + NOT NULL`) + + _, err := f.build(t, "orders", ` + ALTER TABLE %s.orders + ADD COLUMN region_code %s.nonnull_int`) + + assert.ErrorIs(t, err, schemachange.ErrInvariantViolation) + assert.Equal(t, schemachange.CauseStatementNewColumn, schemachange.RefusalCauseOf(err)) + assert.False(t, f.relationExists(t, schemachange.ShadowName(f.schema, "orders")), "a refused build creates nothing") +} + +// Dropping the primary key and declaring a new one under another name +// leaves the shadow without the column every chunk range, watermark, and +// streamed change is keyed on. The new key's own declaration does not lift +// the refusal: the copy is driven by the source's key, which the shadow no +// longer has. +func TestBuildShadowRefusesAStatementThatReplacesThePrimaryKey(t *testing.T) { + f := newShadowFixture(t) + f.createOrders(t) + + _, err := f.build(t, "orders", ` + ALTER TABLE %s.orders + DROP COLUMN id, + ADD COLUMN new_id bigint PRIMARY KEY, + ALTER COLUMN qty TYPE bigint`) + + assert.ErrorIs(t, err, schemachange.ErrInvariantViolation) + assert.Equal(t, schemachange.CauseStatementNewColumn, schemachange.RefusalCauseOf(err)) + assert.False(t, f.relationExists(t, schemachange.ShadowName(f.schema, "orders")), "a refused build creates nothing") +} + +// A new column the server fills — nullable, or NOT NULL with a default — +// builds beside a retype, and stays out of the copy list: the copy writes +// the kept columns and the server fills the new one on every copied row. +func TestBuildShadowAdmitsANewColumnTheServerFills(t *testing.T) { + t.Run("nullable", func(t *testing.T) { + f := newShadowFixture(t) + f.createOrders(t) + + built, err := f.build(t, "orders", ` + ALTER TABLE %s.orders + ADD COLUMN region text, + ALTER COLUMN qty TYPE bigint`) + require.NoError(t, err) + + assert.Equal(t, []string{"id", "qty", "note", "sku"}, built.CopyColumns()) + }) + + t.Run("not null with a default", func(t *testing.T) { + f := newShadowFixture(t) + f.createOrders(t) + + built, err := f.build(t, "orders", ` + ALTER TABLE %s.orders + ADD COLUMN priority integer NOT NULL DEFAULT 0, + ALTER COLUMN qty TYPE bigint`) + require.NoError(t, err) + + assert.Equal(t, []string{"id", "qty", "note", "sku"}, built.CopyColumns()) + }) +} + +// Through the Runner the refusal lands before the run has touched anything: +// the shadow build is the first write, and it rolled back, so there is no +// shadow, no write fence on the source, and no checkpoint row to abandon. +// Without the refusal the run would commit the shadow and abandon itself +// at the first chunk's not_null_violation, reporting a data fault for a +// statement no data could have satisfied. +func TestRunnerRefusesAnUnfillableNewColumnBeforeAnyRowOrFence(t *testing.T) { + f := newRunnerFixture(t) + f.createOrders(t) + + _, err := f.run(t, "orders", ` + ALTER TABLE %s.orders + ADD COLUMN region text NOT NULL, + ALTER COLUMN qty TYPE bigint`, schemachange.RunOptions{}) + + assert.ErrorIs(t, err, schemachange.ErrInvariantViolation) + assert.Equal(t, schemachange.CauseStatementNewColumn, schemachange.RefusalCauseOf(err)) + assert.False(t, f.relationExists(t, schemachange.ShadowName(f.schema, "orders")), "no shadow was committed") + assert.False(t, f.hasFence(t, "orders"), "the source was never fenced") + _, err = f.store.Inspect(t.Context(), f.schema, "orders") + assert.ErrorIs(t, err, checkpoint.ErrNotFound, "the refused run writes no row") + f.assertWritable(t, "orders") +} diff --git a/pkg/schemachange/new_column_test.go b/pkg/schemachange/new_column_test.go new file mode 100644 index 00000000..002cd62f --- /dev/null +++ b/pkg/schemachange/new_column_test.go @@ -0,0 +1,108 @@ +package schemachange + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/schemadiff" +) + +// newColumnFixture is a three-column source — id, qty, note — whose shadow +// kept all three and added one more past them: the added column is a new +// column the copy leaves out, the other three are the source's. +type newColumnFixture struct { + source, shadow []columnPosition +} + +func newNewColumnFixture(added string) newColumnFixture { + source := []columnPosition{{name: "id", attnum: 1}, {name: "qty", attnum: 2}, {name: "note", attnum: 3}} + shadow := append(append([]columnPosition(nil), source...), columnPosition{name: added, attnum: 4}) + return newColumnFixture{source: source, shadow: shadow} +} + +// A new column the copy can leave to the server is not refused here: +// nullable, with a default, generated, or an identity of its own. Each is +// a column the copy omits and the server fills. The identity case is +// refused by the build's new-sequence check instead, which runs after this +// one. +func TestRefuseUnfillableNewColumnsAdmitsAColumnTheServerFills(t *testing.T) { + for name, column := range map[string]schemadiff.Column{ + "nullable": {Name: "sku", Type: "text"}, + "not null, default": {Name: "sku", Type: "text", NotNull: true, Default: "'fresh'::text"}, + "generated": {Name: "sku", Type: "text", NotNull: true, Default: "(qty)::text", Generated: true}, + "identity of its own": {Name: "sku", Type: "bigint", NotNull: true, Identity: schemadiff.IdentityByDefault}, + } { + t.Run(name, func(t *testing.T) { + f := newNewColumnFixture("sku") + shadow := schemadiff.Model{Columns: []schemadiff.Column{{Name: "id"}, {Name: "qty"}, {Name: "note"}, column}} + assert.NoError(t, refuseUnfillableNewColumns("public", "orders", "id", f.source, f.shadow, shadow, nil)) + }) + } +} + +// A new column NOT NULL with no default is refused: the copied rows have no +// value for it and the server would reject the first chunk. The message +// names the column. +func TestRefuseUnfillableNewColumnsRefusesNotNullWithoutADefault(t *testing.T) { + f := newNewColumnFixture("sku") + shadow := schemadiff.Model{Columns: []schemadiff.Column{{Name: "id"}, {Name: "qty"}, {Name: "note"}, {Name: "sku", Type: "text", NotNull: true}}} + + err := refuseUnfillableNewColumns("public", "orders", "id", f.source, f.shadow, shadow, nil) + + require.ErrorIs(t, err, ErrInvariantViolation) + assert.Equal(t, CauseStatementNewColumn, RefusalCauseOf(err)) + assert.ErrorContains(t, err, "sku", "the message names the column") +} + +// A shadow without the source's primary key — dropped outright, with a key +// under a new name in its place — is refused whatever the new key declares: +// the copy is keyed on the source's column and cannot run without it. +func TestRefuseUnfillableNewColumnsRefusesAShadowWithoutThePrimaryKey(t *testing.T) { + source := []columnPosition{{name: "id", attnum: 1}, {name: "qty", attnum: 2}, {name: "note", attnum: 3}} + shadow := []columnPosition{{name: "qty", attnum: 2}, {name: "note", attnum: 3}, {name: "new_id", attnum: 4}} + model := schemadiff.Model{Columns: []schemadiff.Column{{Name: "qty"}, {Name: "note"}, {Name: "new_id", Type: "bigint", NotNull: true, Default: "nextval('orders_new_id_seq'::regclass)"}}} + + err := refuseUnfillableNewColumns("public", "orders", "id", source, shadow, model, nil) + + require.ErrorIs(t, err, ErrInvariantViolation) + assert.Equal(t, CauseStatementNewColumn, RefusalCauseOf(err)) + assert.ErrorContains(t, err, "id", "the message names the key") +} + +// A column the statement kept, retyped, or dropped outright is not a new +// column, however it is declared on the shadow: a kept NOT NULL column with +// no default is the same column, with its values copied. +func TestRefuseUnfillableNewColumnsIgnoresColumnsTheStatementKeptOrDropped(t *testing.T) { + source := []columnPosition{{name: "id", attnum: 1}, {name: "qty", attnum: 2}, {name: "note", attnum: 3}} + shadow := []columnPosition{{name: "id", attnum: 1}, {name: "qty", attnum: 2}} + model := schemadiff.Model{Columns: []schemadiff.Column{{Name: "id", NotNull: true}, {Name: "qty", Type: "bigint", NotNull: true}}} + + assert.NoError(t, refuseUnfillableNewColumns("public", "orders", "id", source, shadow, model, nil)) +} + +// A new NOT NULL column with no default of its own is not refused when its +// domain has a default: a row that leaves the column out takes the +// domain's default, so the copy has a value for it. +func TestRefuseUnfillableNewColumnsAdmitsANotNullColumnWhoseDomainDefaultsIt(t *testing.T) { + f := newNewColumnFixture("sku") + shadow := schemadiff.Model{Columns: []schemadiff.Column{{Name: "id"}, {Name: "qty"}, {Name: "note"}, {Name: "sku", Type: "zero_int", NotNull: true}}} + domains := map[string]domainFill{"sku": {hasDefault: true}} + + assert.NoError(t, refuseUnfillableNewColumns("public", "orders", "id", f.source, f.shadow, shadow, domains)) +} + +// A new column declared nullable is still refused when its domain refuses +// NULL and supplies no default: the first copied row would carry NULL into +// it. +func TestRefuseUnfillableNewColumnsRefusesAColumnWhoseDomainRefusesNull(t *testing.T) { + f := newNewColumnFixture("sku") + shadow := schemadiff.Model{Columns: []schemadiff.Column{{Name: "id"}, {Name: "qty"}, {Name: "note"}, {Name: "sku", Type: "nonnull_int"}}} + domains := map[string]domainFill{"sku": {notNull: true}} + + err := refuseUnfillableNewColumns("public", "orders", "id", f.source, f.shadow, shadow, domains) + + require.ErrorIs(t, err, ErrInvariantViolation) + assert.Equal(t, CauseStatementNewColumn, RefusalCauseOf(err)) +} diff --git a/pkg/schemachange/refusal.go b/pkg/schemachange/refusal.go index c5d4c82b..6198f1b4 100644 --- a/pkg/schemachange/refusal.go +++ b/pkg/schemachange/refusal.go @@ -82,6 +82,14 @@ const ( // the replacement of a source identity column whose sequence the // handoff cannot carry to it. CauseStatementReadd RefusalCause = "shadow-statement-readd" + // CauseStatementNewColumn means the gated statement leaves the shadow + // with a column the copy cannot fill, or without the column the copy + // is keyed on (ST-11): a new column — added beside the other changes, + // not re-added under a dropped column's name — that refuses NULL with + // no default of its own or its domain's for the copied rows to take, or + // a primary key dropped outright or replaced under a new name, so that + // no chunk range or streamed change can name it on the shadow. + CauseStatementNewColumn RefusalCause = "shadow-statement-new-column" // CauseStatementNewSequence means the gated statement gives the shadow // an identity or sequence-owned column the source does not have — an // added identity or serial column, or a kept column made an identity @@ -257,6 +265,7 @@ func RefusalCauses() []RefusalCause { CauseGrantsDiffer, CauseIdentityHandoff, CauseStatementReadd, + CauseStatementNewColumn, CauseStatementNewSequence, CauseCutoverUnverified, CauseCutoverUndrained, @@ -303,7 +312,7 @@ func (c RefusalCause) Invariant() string { return "LK-3" case CauseStatementTarget, CauseStatementConversion: return "ST-7" - case CauseStatementReadd: + case CauseStatementReadd, CauseStatementNewColumn: return "ST-11" case CauseShadowOwner, CauseForeignRelation, CauseGrantsDiffer, CauseIdentityHandoff, CauseSchemaDrift, CauseFidelityDrift, CauseIndexInvalid, CauseNameTaken: @@ -330,8 +339,8 @@ func (c RefusalCause) Invariant() string { // a lock, a proof, a relation, a slot, or a row whose state moved under the // operation is environmental — state outside the engine stopped a // supportable change. A statement the engine declines to carry as written -// — a conversion the copy cannot reproduce, a re-added column it cannot -// fill, a new identity or sequence the swap cannot carry — is a capability +// — a conversion the copy cannot reproduce, a re-added or new column it +// cannot fill, a new identity or sequence the swap cannot carry — is a capability // boundary. Every other cause is an invariant violation: an // input the caller handed in that cannot have come from this engine, or a // write of the engine's own that the catalog does not show; nothing on the @@ -351,7 +360,7 @@ func (c RefusalCause) Class() refusal.Class { CauseResumeShadowGone, CauseResumeSlotForeign, CauseResumeSlotHeld, CauseRestartSwapCommitted, CauseCopyConversionDrift: return refusal.ClassEnvironmental - case CauseStatementConversion, CauseStatementReadd, CauseStatementNewSequence: + case CauseStatementConversion, CauseStatementReadd, CauseStatementNewColumn, CauseStatementNewSequence: return refusal.ClassCapabilityBoundary case CauseLockUnproven, CauseProofEmpty, CauseStatementTarget, CauseGrantsDiffer, CauseCutoverUnverified, CauseCutoverUndrained, CauseSwapMismatch, CauseResumeTerminal, diff --git a/pkg/schemachange/refusal_test.go b/pkg/schemachange/refusal_test.go index b4646dd4..dd4947de 100644 --- a/pkg/schemachange/refusal_test.go +++ b/pkg/schemachange/refusal_test.go @@ -34,6 +34,7 @@ func TestEveryRefusalCauseNamesItsInvariant(t *testing.T) { CauseGrantsDiffer: "ST-5", CauseIdentityHandoff: "ST-5", CauseStatementReadd: "ST-11", + CauseStatementNewColumn: "ST-11", CauseStatementNewSequence: "ST-6", CauseSchemaDrift: "ST-5", CauseFidelityDrift: "ST-5", @@ -91,6 +92,7 @@ func TestEveryRefusalCauseNamesItsClass(t *testing.T) { CauseGrantsDiffer: refusal.ClassInvariantViolation, CauseIdentityHandoff: refusal.ClassEnvironmental, CauseStatementReadd: refusal.ClassCapabilityBoundary, + CauseStatementNewColumn: refusal.ClassCapabilityBoundary, CauseStatementNewSequence: refusal.ClassCapabilityBoundary, CauseCutoverUnverified: refusal.ClassInvariantViolation, CauseCutoverUndrained: refusal.ClassInvariantViolation, diff --git a/pkg/schemachange/shadow.go b/pkg/schemachange/shadow.go index 923f885f..08f9033f 100644 --- a/pkg/schemachange/shadow.go +++ b/pkg/schemachange/shadow.go @@ -307,6 +307,13 @@ func buildShadow(ctx context.Context, pool *pgxpool.Pool, lock *dbconn.TableLock if err := refuseUnfillableReadds(target.Schema(), target.Table(), target.PKColumn(), sourcePositions, shadowPositions, targetModel, shadowDomains, identities); err != nil { return BuiltShadow{}, err } + // Any other column the copy list leaves out is a new column the server + // fills; one it cannot fill, or a key the shadow no longer has, is + // refused here rather than by the first chunk's error after the shadow + // is committed. + if err := refuseUnfillableNewColumns(target.Schema(), target.Table(), target.PKColumn(), sourcePositions, shadowPositions, targetModel, shadowDomains); err != nil { + return BuiltShadow{}, err + } // A sequence the statement minted on the shadow has no place in the // swap's handoff, which carries the source's sequences and identity // columns alone; it is refused here, before a copy would run to the