diff --git a/.agents/checks/review.md b/.agents/checks/review.md index a032ffa..af8fee7 100644 --- a/.agents/checks/review.md +++ b/.agents/checks/review.md @@ -16,7 +16,7 @@ the reviewer's distillation. - Core packages: every loop, queue, retry, and wait must be bounded. An unbounded anything in a core package is a review-blocking defect. - Dangerous APIs accept proof types (`statement.Classified`, `PreflightedTable`, - `AbsentTarget`, `VerifiedShadow`, `CleanWatermark`, `TableLock`) with package-private + `AbsentTarget`, `CreationRole`, `VerifiedShadow`, `CleanWatermark`, `TableLock`) with package-private constructors — never a raw string or bool that a caller could fabricate. Core code re-verifies its own preconditions; it never trusts that the planner or CLI checked. diff --git a/README.md b/README.md index 64cefcb..69431ba 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,9 @@ refusal — never a silently wrong or incomplete result: - **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. +- **Greenfield `CREATE TABLE` apply** is not user-reachable yet: the + executor create path exists as a library building block, but the + declarative front door does not route to it. - **Non-table objects** — views, standalone sequences, enums, domains, extensions, functions, triggers — are outside the declarative model, which covers one ordinary table plus its indexes per file. diff --git a/SAFETY.md b/SAFETY.md index f58d66f..d2a7bc8 100644 --- a/SAFETY.md +++ b/SAFETY.md @@ -67,8 +67,8 @@ The short version — the full rules live in [docs/tcb-model.md](docs/tcb-model. - **Never trust callers.** Every dangerous operation re-verifies its preconditions, whoever the requester is (CLI, planner, orchestrator). The periphery may request; the core enforces. - **Domain types make illegal states unrepresentable.** Validating passages return proof types - with package-private constructors (today `preflight.PreflightedTable` and - `preflight.AbsentTarget`; later phases add + with package-private constructors (today `preflight.PreflightedTable`, + `preflight.AbsentTarget`, and `preflight.CreationRole`; later phases add `VerifiedShadow`, `CleanWatermark`, and `TableLock`); dangerous APIs accept only proof types — e.g. the planned cutover swap will accept only a `VerifiedShadow`. - **Put a limit on everything.** Every loop bounded, every queue bounded, every retry counted, diff --git a/docs/capabilities.md b/docs/capabilities.md index f12d86d..bc5fe79 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -171,7 +171,7 @@ Status legend: ✅ T1 (supported today) · 🟡 T2 (planned; typed refusal today | Unlogged tables | 🟡 | Yes | Typed refusal: persistence is not modeled, converging it (`SET LOGGED`) is a full rewrite, and rendering the table as plain `CREATE TABLE` would silently change crash-safety | | Explicit column collations | 🟡 | Yes | Typed refusal: dropping a `COLLATE` clause from a rendered baseline silently changes sort order and index semantics; a collation delta cannot converge without a rewrite | | Columns whose default uses a sequence the column does not own | 🟡 | Yes | Typed refusal: in a desired-state model that sequence exists only inside the scratch transaction, so no derived plan can reference it. Column-owned (`serial`-style) sequences are fine | -| Greenfield `CREATE TABLE` apply (the table does not exist yet — a fresh database or a new table in a live one) | 🟡 | Yes — a `REFERENCES` clause takes a brief `SHARE ROW EXCLUSIVE` on each **referenced** live table | Planned as an owned operation. The new table itself has no readers or writers to protect; the online-safety problem is the `REFERENCES` clause, whose lock on each referenced live table queues behind long-running queries and blocks writers behind it — exactly the run-it-under-a-bounded-`lock_timeout` job this engine owns and owner tooling does not do. The absence preflight (`CheckTableAbsent`) is in place; the executor create path and front-door admission build on it. `diff --sql` already emits the statement | +| Greenfield `CREATE TABLE` apply (the table does not exist yet — a fresh database or a new table in a live one) | 🟡 | Yes — a `REFERENCES` clause takes a brief `SHARE ROW EXCLUSIVE` on each **referenced** live table | Planned as an owned operation. The new table itself has no readers or writers to protect; the online-safety problem is the `REFERENCES` clause, whose lock on each referenced live table queues behind long-running queries and blocks writers behind it — exactly the run-it-under-a-bounded-`lock_timeout` job this engine owns and owner tooling does not do. The absence preflight (`CheckTableAbsent`), the creation-privilege preflight (`CheckCreatePrivileges`), and the executor create path (`ExecuteCreate` — plain `CREATE TABLE` plus plain index builds; `PARTITION OF`, `INHERITS`, `LIKE`, `OF`, and `IF NOT EXISTS` are typed refusals at admission, while `REFERENCES` and `CONCURRENTLY` are refused upstream at desired-file parse and re-checked at admission as defense in depth) are in place; the declarative front door does not route to them yet. `diff --sql` already emits the statement | ### Types and non-table objects diff --git a/docs/engine-role.md b/docs/engine-role.md index b3b977b..c92740a 100644 --- a/docs/engine-role.md +++ b/docs/engine-role.md @@ -55,6 +55,15 @@ same preflight: `wal_level = logical` (`rds.logical_replication = 1` on Aurora/R static parameter requiring a reboot), and free `max_replication_slots` / `max_wal_senders` headroom. +### Off-ladder: greenfield `CREATE TABLE` + +Creating a new table sits outside the ladder: the table does not exist yet, so there is no +owning role to be a member of — the table is born owned by the role that creates it. The +create path's preflight (`CheckCreatePrivileges`) therefore proves exactly `CONNECT` on the +database plus `USAGE` and `CREATE` on the target schema, deliberately not the Tier 1–3 +ownership membership. A missing grant is refused with the exact `GRANT` statement, whose +grantee is the engine role itself. + ## Provisioning For a target whose tables are owned by `app_owner` in schema `app`: diff --git a/docs/execution-model.md b/docs/execution-model.md index 034ebbf..d003406 100644 --- a/docs/execution-model.md +++ b/docs/execution-model.md @@ -21,6 +21,7 @@ and [suggest-report.md](suggest-report.md#caveats-caveats). - [The committed prefix](#the-committed-prefix) - [How a failure is reported](#how-a-failure-is-reported) - [Why the prefix is safe to leave](#why-the-prefix-is-safe-to-leave) +- [Outcome codes](#outcome-codes) ## Why there is no wrapping transaction @@ -241,3 +242,37 @@ statement — stopping at the first refusal or failure. Its result carries the plan, one verdict per attempted statement, and a detail naming exactly which planned statements committed and remain in effect: the committed prefix at the plan level, statements instead of steps. + +## Outcome codes + +`executor.Codes()` enumerates the closed vocabulary below, and +`executor.OutcomeCode` maps any executor error to its entry — the same code +that reaches the JSON verdict's `code` field. Adapters render three facts +per failure — the outcome code, the failing step's position +(`SequenceStepError.Step` of `.Total`), and the failing step's SQL — and +log the raw error, whose text interpolates server prose and is not a +branching surface. + +| Code | Meaning | +| --- | --- | +| `budget-lock-exceeded` | The lock was not granted within `lock_timeout`; nothing executed | +| `budget-statement-exceeded` | The statement ran past `statement_timeout` and was cancelled | +| `cancelled-externally` | The statement was cancelled from outside the executor before its budget elapsed | +| `invalid-index-own-leftover` | The failed build's own INVALID index remains; the [recovery runbook](invalid-index-recovery.md) applies | +| `invalid-index-preexisting` | An INVALID index under the requested name predates this run | +| `invalid-index-unproven` | An INVALID index may remain but the catalog state could not be proven | +| `empty-sequence` | The sequence had no steps to run | +| `unsupported-sequence-step` | A step is not a shape the sequence executor can run safely | +| `unsupported-partitioned-parent` | Partitioned-parent admission refusal | +| `not-concurrent-index-build` | The statement handed to the concurrent build executor is not a `CREATE INDEX CONCURRENTLY` | +| `unnamed-index` | The concurrent build does not name its index, so its outcome could not be verified | +| `unqualified-table` | The target table is not schema-qualified at the library boundary | +| `if-not-exists-unsupported` | `CREATE ... IF NOT EXISTS` cannot prove what its no-op would mean | +| `create-collision` | A name the create path needs is already taken on the server; re-diff the live catalog | +| `duplicate-create-name` | The desired set claims the same relation name twice; refused at admission | +| `partition-of-unsupported` | `CREATE TABLE PARTITION OF` locks the partitioned parent, which the absence proof does not cover | +| `unsupported-create-step` | A desired statement is not a shape the create path can run | +| `pool-too-small` | The pool cannot hold the build session and the verdict connection at once | +| `table-not-found` | The statement's qualified table does not exist | +| `invariant-violation` | A breach of the invariant registry; never a retry candidate | +| `execution-failed` | Fallback for a failure outside the typed set — an operational error to investigate, not a refusal to branch on | diff --git a/docs/invariants.md b/docs/invariants.md index ff2f0d7..b75308c 100644 --- a/docs/invariants.md +++ b/docs/invariants.md @@ -244,8 +244,9 @@ executes, any statement whose target table does not match the preflight proof it A proof for one table can never smuggle SQL against another, and a multi-statement string can never reach the database through the executor (pgx's simple protocol would happily run all of it). *Enforced:* `pkg/executor` (`ExecuteNative`; `RunSequence` admission re-proves every step's -target against the preflight proof before the first step executes), `pkg/statement` (proof -construction). +target against the preflight proof before the first step executes; `ExecuteCreate` re-proves +every desired statement's target against the absence proof the same way), `pkg/statement` +(proof construction). *Source:* adversarial review of the optimistic front door. ## Refusals and preflight (RF) diff --git a/docs/limitations.md b/docs/limitations.md index 92cae4e..202e0b1 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -29,6 +29,7 @@ with a typed refusal — never a silently wrong or incomplete result: | Column collations | An explicit `COLLATE` on a column is not managed: converging a collation delta rewrites the column and its indexes. Export refuses a collated column — a baseline without the clause would silently change sort order and index semantics — and a collation delta (including on an added column) is a typed `diff` refusal. | | Non-table objects | Views, materialized views, standalone sequences, enums, domains, extensions, functions, and triggers are outside the model. A serial column's owned sequence is the one exception: it round-trips through the `serial` pseudo-types — and ownership is verified through the catalog (`pg_depend`), so a hand-written `nextval` default on a standalone sequence that merely carries the serial-style name refuses rather than exporting as `serial` and silently privatizing a shared sequence. A column may *use* an unmanaged type (an enum, a domain) — the type text round-trips — but the type's definition is not managed. | | Multiple tables per file | A desired file is single-table scoped: exactly one `CREATE TABLE` plus `CREATE INDEX` statements on it. Multi-table schemas are managed as one file per table. | +| Greenfield table creation | Not user-reachable yet: the executor create path (`executor.ExecuteCreate`) exists as a library building block, but the declarative front door does not route to it. The path runs a plain `CREATE TABLE` plus plain index builds on the table born that run, and refuses at admission — before anything executes — every clause that binds to an existing object the absence proof does not cover: `PARTITION OF`, `INHERITS`, `LIKE`, and `OF`, plus `IF NOT EXISTS`. `REFERENCES` and `CONCURRENTLY` are refused upstream at desired-file parse (`statement.ParseDesired`); the create path's admission re-checks them as defense in depth. | | Changed index or constraint definition | A redefinition diffs to drop-and-recreate, the drop is destructive, and desired-state execution refuses any plan containing a destructive statement — the whole plan, including the harmless recreate. Run the drop deliberately first (`DROP INDEX CONCURRENTLY` directly against the database; `ALTER TABLE ... DROP CONSTRAINT` through the imperative front door), then rerun — the remaining plan converges the recreate. | ## What desired-state execution converges today diff --git a/docs/schemabot-integration.md b/docs/schemabot-integration.md index b986f22..773191b 100644 --- a/docs/schemabot-integration.md +++ b/docs/schemabot-integration.md @@ -143,22 +143,46 @@ package. Landing this is one of: ### Routing the create path's refusals -The planned greenfield `CREATE TABLE` path opens with `preflight.CheckTableAbsent`, and its -proof has a rule the adapter must respect: an `AbsentTarget` is **minted inside the apply +The greenfield `CREATE TABLE` path is a fixed call order, all inside the apply session: + +1. `statement.ParseDesired` — parse and validate the desired file (refuses `REFERENCES`, + `CONCURRENTLY`, qualified names). +2. `preflight.CheckCreatePrivileges` — mint the `CreationRole` proof for the target schema. +3. `preflight.CheckTableAbsent` — mint the `AbsentTarget` proof for the table name. +4. `executor.ExecuteCreate` — consume both proofs and run the set. + +Both proofs share one rule the adapter must respect: they are **minted inside the apply session and consumed there** — never serialized into `SchemaChange.Metadata`, carried across -the plan/apply boundary, or reused across retries. Absence at plan time proves nothing about -apply time; the executor re-verifies inside the session that runs the `CREATE`, the same way -ST-7 re-verifies a `PreflightedTable`. +the plan/apply boundary, or reused across retries. Absence or privilege at plan time proves +nothing about apply time; the executor re-verifies inside the session that runs the +`CREATE`, the same way ST-7 re-verifies a `PreflightedTable`. -Each refusal from the check maps to a different orchestrator action — route them, don't -retry them uniformly: +Each refusal from the preflight checks maps to a different orchestrator action — route +them, don't retry them uniformly: | Refusal | What it means | Orchestrator action | | --- | --- | --- | | `ErrRelationExists` / `ErrTypeExists` (grouped by `preflight.IsNameOccupied`) | The name is already taken — this is not a create, it's a change to something that exists | Route to the diff/alter path, not to a failure state | | `ErrSchemaNotFound` | The qualified schema does not exist on the target | Operator action (create the schema or fix the desired file); retrying cannot succeed | | `ErrNoCreationSchema` | Unqualified name and the role's `search_path` yields no creation schema | Caller configuration: schema-qualify the name or fix the role's `search_path` | -| Duplicate-name error from the `CREATE` itself | A concurrent writer won the race after a valid proof | Re-plan from scratch — the world changed; do not blindly retry the create | +| `*preflight.PrivilegeError` (`Tier == TierCreateTable`) | The role lacks `CREATE` on the schema (or `USAGE` reaching it); the error carries the exact missing grant | Operator action: provision the named `GRANT`, then retry | + +`ExecuteCreate`'s own refusals and failures carry the same routing discipline +([outcome codes](execution-model.md#outcome-codes)): + +| Outcome | What it means | Orchestrator action | +| --- | --- | --- | +| `ErrDuplicateCreateName` (`duplicate-create-name`) | The desired set claims one relation name twice — including a first-choice implicit constraint-index name; refused at admission, nothing ran | Fix the desired file; retrying unchanged cannot succeed | +| `ErrPartitionOfUnsupported` (`partition-of-unsupported`) | `PARTITION OF` binds to a live parent the absence proof does not cover | Fix the desired file; out of the create path's scope | +| `ErrUnsupportedCreateStep` (`unsupported-create-step`) | A desired statement is not a shape the create path can run | Fix the desired file | +| `ErrCreateCollision` (`create-collision`) | A concurrent writer took a needed name after a valid proof | Re-diff the live catalog and re-plan — the world changed; never blindly retry the create | + +A failed create is not rolled back wholesale: each step committed in its own bounded +transaction, so the steps before the failure remain +([the committed prefix](execution-model.md#the-committed-prefix)). A rerun's absence check +then refuses with `ErrRelationExists`, and the gate stays closed until the declarative +front door re-diffs the live catalog and converges the remainder — the orchestrator never +assumes the failed run left nothing behind. ## Execution-mode verdicts and direct execution diff --git a/docs/tcb-model.md b/docs/tcb-model.md index ac54fe3..0daa056 100644 --- a/docs/tcb-model.md +++ b/docs/tcb-model.md @@ -92,6 +92,7 @@ to obtain the type is through the function that validates it. | `string` (user SQL) | `statement.ParseOne` / `statement.ParseOps`, then `planner.Classify` | `planner.Plan` / `planner.Decision` | CO-7 — classification consumes parsed operation descriptors | | table name | preflight | `PreflightedTable` (carries the proven facts: PK, no FKs/views, replica identity, headroom) | ST-6, RF-* | | table name (create target) | `preflight.CheckTableAbsent` | `AbsentTarget` (carries the resolved creation schema and the verified-free name; time-of-check — minted inside the apply session, never carried across a plan boundary, and re-verified at use the way ST-7 re-verifies `PreflightedTable`) | ST-6 for the create path | +| creating role's access (create target) | `preflight.CheckCreatePrivileges` | `CreationRole` (carries the connected role and the resolved creation schema whose CONNECT / USAGE / CREATE grants were verified; time-of-check and session-scoped, like `AbsentTarget` — a revoked grant after minting fails with the server's own error) | ST-6 for the create path | | shadow table | full checksum pass (planned) | `VerifiedShadow` — its constructor will be private to `pkg/checksum`; the planned `cutover.Swap` will accept **only** this type | CO-1 in the type system | | chunker low-watermark | all-checkers-clean pass (planned) | `CleanWatermark` — will be unobtainable in a pass that repaired anything | CO-2 | | — | planned table-lock acquisition | `TableLock` token, planned as a required parameter of every mutating operation | LK-1 | diff --git a/pkg/executor/code.go b/pkg/executor/code.go index 7780812..1bbe3a7 100644 --- a/pkg/executor/code.go +++ b/pkg/executor/code.go @@ -53,9 +53,23 @@ const ( // CodeUnqualifiedTable: the target table is not schema-qualified at // the library boundary. CodeUnqualifiedTable Code = "unqualified-table" - // CodeIfNotExistsUnsupported: CREATE INDEX CONCURRENTLY IF NOT EXISTS - // cannot prove what its no-op would mean. + // CodeIfNotExistsUnsupported: CREATE ... IF NOT EXISTS cannot prove + // what its no-op would mean. CodeIfNotExistsUnsupported Code = "if-not-exists-unsupported" + // CodeCreateCollision: a name the create path needs is already taken + // on the server; the caller re-diffs the live catalog rather than + // assuming the occupant's shape. + CodeCreateCollision Code = "create-collision" + // CodeDuplicateCreateName: the desired set claims the same relation + // name twice; the conflict is decidable at admission and refused + // before anything runs. + CodeDuplicateCreateName Code = "duplicate-create-name" + // CodePartitionOfUnsupported: CREATE TABLE PARTITION OF locks the + // partitioned parent, which the absence proof does not cover. + CodePartitionOfUnsupported Code = "partition-of-unsupported" + // CodeUnsupportedCreateStep: a desired statement is not a shape the + // create path can run. + CodeUnsupportedCreateStep Code = "unsupported-create-step" // CodePoolTooSmall: the pool cannot hold the build session and the // verdict connection at once. CodePoolTooSmall Code = "pool-too-small" @@ -71,6 +85,36 @@ const ( CodeExecutionFailed Code = "execution-failed" ) +// Codes returns the closed set of outcome codes. It is part of the report +// contract: adapters enumerate it to know every outcome they must render, +// and the docs test pins every code into the execution-model page so the +// documented vocabulary cannot drift from this one. +func Codes() []Code { + return []Code{ + CodeBudgetLockExceeded, + CodeBudgetStatementExceeded, + CodeCancelledExternally, + CodeInvalidIndexOwnLeftover, + CodeInvalidIndexPreexisting, + CodeInvalidIndexUnproven, + CodeEmptySequence, + CodeUnsupportedSequenceStep, + CodeUnsupportedPartitionedParent, + CodeNotConcurrentIndexBuild, + CodeUnnamedIndex, + CodeUnqualifiedTable, + CodeIfNotExistsUnsupported, + CodeCreateCollision, + CodeDuplicateCreateName, + CodePartitionOfUnsupported, + CodeUnsupportedCreateStep, + CodePoolTooSmall, + CodeTableNotFound, + CodeInvariantViolation, + CodeExecutionFailed, + } +} + // OutcomeCode maps an error returned by this package to its stable code. // A nil error has no outcome code and maps to the empty Code. A // *SequenceStepError carries its failed step's own cause, so it maps to @@ -119,6 +163,14 @@ func sentinelCode(err error) Code { return CodeUnqualifiedTable case errors.Is(err, ErrIfNotExistsUnsupported): return CodeIfNotExistsUnsupported + case errors.Is(err, ErrCreateCollision): + return CodeCreateCollision + case errors.Is(err, ErrDuplicateCreateName): + return CodeDuplicateCreateName + case errors.Is(err, ErrPartitionOfUnsupported): + return CodePartitionOfUnsupported + case errors.Is(err, ErrUnsupportedCreateStep): + return CodeUnsupportedCreateStep case errors.Is(err, ErrPoolTooSmall): return CodePoolTooSmall case errors.Is(err, ErrTableNotFound): diff --git a/pkg/executor/code_test.go b/pkg/executor/code_test.go index 6c497f1..84745ea 100644 --- a/pkg/executor/code_test.go +++ b/pkg/executor/code_test.go @@ -52,6 +52,10 @@ func TestOutcomeCodeMapsTypedOutcomes(t *testing.T) { {name: "unnamed index", err: executor.ErrUnnamedIndex, want: executor.CodeUnnamedIndex}, {name: "unqualified table", err: executor.ErrUnqualifiedTable, want: executor.CodeUnqualifiedTable}, {name: "if not exists", err: executor.ErrIfNotExistsUnsupported, want: executor.CodeIfNotExistsUnsupported}, + {name: "create collision", err: executor.ErrCreateCollision, want: executor.CodeCreateCollision}, + {name: "duplicate create name", err: executor.ErrDuplicateCreateName, want: executor.CodeDuplicateCreateName}, + {name: "partition of", err: executor.ErrPartitionOfUnsupported, want: executor.CodePartitionOfUnsupported}, + {name: "unsupported create step", err: executor.ErrUnsupportedCreateStep, want: executor.CodeUnsupportedCreateStep}, {name: "pool too small", err: executor.ErrPoolTooSmall, want: executor.CodePoolTooSmall}, {name: "table not found", err: executor.ErrTableNotFound, want: executor.CodeTableNotFound}, {name: "invariant violation", err: executor.ErrInvariantViolation, want: executor.CodeInvariantViolation}, diff --git a/pkg/executor/create.go b/pkg/executor/create.go new file mode 100644 index 0000000..7187d64 --- /dev/null +++ b/pkg/executor/create.go @@ -0,0 +1,308 @@ +// This file is the create-path executor: it runs a validated desired +// schema — one CREATE TABLE plus its indexes — against a name the caller +// proved absent. Every step is a brief bounded transactional run: the +// table is born this run and carries no traffic, so its indexes are built +// plainly rather than CONCURRENTLY — a plain build on an empty table is +// fast, and unlike CONCURRENTLY it cannot leave an INVALID index behind a +// failure. The executor never trusts the caller's classification (see +// SAFETY.md): each desired statement is qualified into the proof's schema, +// re-parsed by the real grammar, and admitted by shape and target before +// anything executes. +// +// The absence proof is time-of-check: nothing locks the name, so a +// concurrent create can still take it between the check and a step here. +// That loss surfaces as SQLSTATE 42P07 and is returned as the typed +// ErrCreateCollision — the caller re-diffs the live catalog rather than +// assuming what the collision left behind. A failed step ends the run +// immediately; the steps before it committed (each in its own bounded +// transaction) and remain, so a rerun's absence check refuses with +// ErrRelationExists and the declarative front door re-diffs and applies +// the remainder. + +package executor + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/jackc/pgx/v5/pgconn" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/block/pg-sprite/pkg/preflight" + "github.com/block/pg-sprite/pkg/progress" + "github.com/block/pg-sprite/pkg/statement" +) + +// Typed refusals and failures for the create path. Admission covers every +// desired statement before the first executes, so a creation this executor +// cannot finish is never started. +var ( + // ErrCreateCollision is returned when a step fails because its target + // name is already taken. For the table name that means a concurrent + // create won the race — the absence proof is time-of-check. Index + // names are never absence-checked, so a pre-existing occupant at an + // index name reports the same way. Either way the caller re-diffs the + // live catalog; nothing about the occupant's shape can be assumed. + ErrCreateCollision = errors.New("a name the create path needs is already taken") + // ErrDuplicateCreateName is returned when the desired set claims the + // same relation name twice — two indexes under one name, or an index + // named after the table. The conflict is decidable before anything + // runs, so admission refuses the whole set rather than letting a + // mid-run step fail after a prefix committed. + ErrDuplicateCreateName = errors.New("desired set claims the same relation name twice") + // ErrPartitionOfUnsupported is returned for CREATE TABLE ... PARTITION + // OF: attaching a partition takes a lock on the partitioned parent, + // an existing table the absence proof says nothing about. + ErrPartitionOfUnsupported = errors.New("CREATE TABLE PARTITION OF is not supported by the create path: attaching a partition locks the partitioned parent, which the absence proof does not cover") + // ErrUnsupportedCreateStep is returned when a desired statement is not + // a shape the create path can run: a plain CREATE TABLE or a plain + // CREATE INDEX on the new table. CONCURRENTLY is refused deliberately — + // the table is born this run with no traffic to protect, and a plain + // build cannot leave an INVALID index behind a failure. + ErrUnsupportedCreateStep = errors.New("statement is not a shape the create path can run") +) + +// The SQLSTATEs a create step raises when its target name is already +// taken. Postgres errors are matched by SQLSTATE, never by message text. +const ( + // sqlstateDuplicateTable: the occupant is a relation — any kind, an + // index included. + sqlstateDuplicateTable = "42P07" + // sqlstateDuplicateObject: the occupant is not a relation — a + // standalone type under the table's name raises it, because every + // table also mints a composite type of the same name. + sqlstateDuplicateObject = "42710" +) + +// ExecuteCreate runs the desired schema's statements against the +// verified-absent target: the CREATE TABLE first, then its indexes in +// input order, each step a bounded transactional run under the brief +// budgets, exactly like an optimistic attempt, with its search_path +// pinned to the proof's schema then public — the same policy the +// introspection read path sets — so the desired file's unqualified +// references resolve exactly as the diff resolved them. The pool must +// come from pkg/dbconn and must be the session the proofs were minted on: +// cr proves that session's role can create in the proof's schema, and +// like the absence proof it is time-of-check — a grant revoked after +// minting fails with the server's own error. Every desired statement is +// qualified into the proof's schema, re-parsed, and admitted by shape and +// target before the first step executes. On success every step committed +// and the report says what each did. On failure the run stops at the +// failing step and returns a typed *SequenceStepError; the committed +// prefix remains — a rerun's absence check then refuses with +// preflight.ErrRelationExists, and the caller re-diffs the live catalog +// to apply the remainder. retry bounds lock_timeout retries on each step, +// exactly as in ExecuteNative. +func ExecuteCreate(ctx context.Context, pool *pgxpool.Pool, at preflight.AbsentTarget, cr preflight.CreationRole, ds statement.DesiredSchema, b Budget, retry RetryPolicy) (SequenceReport, error) { + return executeCreate(ctx, pool, at, cr, ds, b, retry, nil) +} + +// ExecuteCreateWithProgress runs the create path while updating tracker +// with the current step. The caller may poll concurrently. +func ExecuteCreateWithProgress(ctx context.Context, pool *pgxpool.Pool, at preflight.AbsentTarget, cr preflight.CreationRole, ds statement.DesiredSchema, b Budget, retry RetryPolicy, tracker *progress.Tracker) (rep SequenceReport, err error) { + if tracker == nil { + return rep, fmt.Errorf("%w: progress tracker is required", ErrInvariantViolation) + } + tracker.Start(len(ds.Statements()), progress.OperationAdmitting) + defer func() { tracker.Finish(err) }() + return executeCreate(ctx, pool, at, cr, ds, b, retry, tracker) +} + +func executeCreate(ctx context.Context, pool *pgxpool.Pool, at preflight.AbsentTarget, cr preflight.CreationRole, ds statement.DesiredSchema, b Budget, retry RetryPolicy, tracker *progress.Tracker) (SequenceReport, error) { + var rep SequenceReport + if err := b.validate(); err != nil { + return rep, err + } + if err := retry.validate(); err != nil { + return rep, err + } + // INV: ST-7 — the proofs are re-verified at the point of use. Zero + // values are forgeable by any package: only CheckTableAbsent mints an + // AbsentTarget with a table, only CheckCreatePrivileges mints a + // CreationRole with a schema, and only ParseDesired mints a + // DesiredSchema with a table. + if at.Schema() == "" || at.Table() == "" { + return rep, fmt.Errorf("%w: ST-7: absence proof carries no verified target", ErrInvariantViolation) + } + if cr.Schema() == "" { + return rep, fmt.Errorf("%w: ST-7: creation-privilege proof carries no verified schema", ErrInvariantViolation) + } + if cr.Schema() != at.Schema() { + return rep, fmt.Errorf("%w: ST-7: creation privileges were verified in %q but absence in %q", + ErrInvariantViolation, cr.Schema(), at.Schema()) + } + if ds.Table() == "" { + return rep, fmt.Errorf("%w: ST-7: desired schema carries no admitted CREATE TABLE", ErrInvariantViolation) + } + if ds.Table() != at.Table() { + return rep, fmt.Errorf("%w: ST-7: desired schema targets %q but absence was verified for %q", + ErrInvariantViolation, ds.Table(), at.Table()) + } + steps, err := admitCreateSteps(at, ds) + if err != nil { + return rep, err + } + for i, step := range steps { + start := time.Now() + if tracker != nil { + tracker.StartStep(i+1, progress.OperationBrief) + start = tracker.Now() + } + err := executeWithLockRetryObserved(ctx, retry, func(ctx context.Context) error { + return executeBoundedAttempt(ctx, pool, step, b, at.Schema()) + }, sleepContext, func(attempt int) { + if tracker != nil { + tracker.SetAttempt(attempt) + } + }) + if err != nil { + return rep, &SequenceStepError{Step: i + 1, Total: len(steps), Kind: StepBrief, SQL: step.SQL(), Err: asCreateCollision(err)} + } + rep.Steps = append(rep.Steps, StepReport{ + SQL: step.SQL(), + Kind: StepBrief, + Duration: elapsedSince(tracker, start), + }) + } + return rep, nil +} + +// admitCreateSteps qualifies every desired statement into the proof's +// schema, re-parses it, and admits it by shape and target. The CREATE +// TABLE is ordered first regardless of its input position — an index +// cannot be built before its table exists — and the indexes keep their +// input order after it. Every step claims the names it will occupy in the +// same pg_class namespace — the table plus the first-choice index names +// of its index-backed constraints, or an explicit index name — so a name +// claimed twice within the set — decidable here — is refused before +// anything runs rather than failing mid-run after a prefix committed. +// The claims are first choices: a set whose first choices collide is +// refused even where the server would sidestep with a numeric suffix, +// because a deterministic name the file states beats one the server +// invents. A step whose name the server invents outright (an unnamed +// index) claims nothing decidable and is exempt. +func admitCreateSteps(at preflight.AbsentTarget, ds statement.DesiredSchema) ([]statement.Statement, error) { + desired := ds.Statements() + var createStep statement.Statement + var haveCreate bool + indexSteps := make([]statement.Statement, 0, len(desired)) + claimed := make(map[string]struct{}, len(desired)) + for i, raw := range desired { + st, names, err := admitCreateStep(at, raw.SQL()) + if err != nil { + return nil, fmt.Errorf("desired statement %d of %d: %w", i+1, len(desired), err) + } + for _, name := range names { + if _, taken := claimed[name]; taken { + return nil, fmt.Errorf("desired statement %d of %d: %w: %q", i+1, len(desired), ErrDuplicateCreateName, name) + } + claimed[name] = struct{}{} + } + if st.Kind() == statement.KindCreateTable { + createStep = st + haveCreate = true + continue + } + indexSteps = append(indexSteps, st) + } + if !haveCreate { + // A DesiredSchema proof guarantees exactly one CREATE TABLE; a + // set without one here means the proof was forged or mutated. + return nil, fmt.Errorf("%w: ST-7: desired schema admitted without a CREATE TABLE", ErrInvariantViolation) + } + return append([]statement.Statement{createStep}, indexSteps...), nil +} + +// admitCreateStep qualifies one desired statement into the proof's schema, +// re-parses it by the real grammar, and admits it by shape and target. It +// returns the pg_class names the step will claim — for a CREATE TABLE the +// table name plus the first-choice index names of its index-backed +// constraints, for a CREATE INDEX its explicit name, nothing when the +// server invents one. CREATE TABLE clauses that bind to a secondary +// relation or type — PARTITION OF, INHERITS, LIKE, OF — are refused: +// statement.Qualify rewrites only the target, so the secondary name would +// resolve via search_path to an existing object the absence proof says +// nothing about. +func admitCreateStep(at preflight.AbsentTarget, sql string) (statement.Statement, []string, error) { + qualified, err := statement.Qualify(sql, at.Schema()) + if err != nil { + return statement.Statement{}, nil, err + } + st, err := statement.ParseOne(qualified) + if err != nil { + return statement.Statement{}, nil, err + } + ops, err := statement.ParseOps(qualified) + if err != nil { + return statement.Statement{}, nil, err + } + if len(ops) != 1 { + // ParseOne admitted a single statement, so a differing op count + // means the two parse boundaries disagree about the same SQL. + return statement.Statement{}, nil, fmt.Errorf("%w: statement carries %d operations", ErrUnsupportedCreateStep, len(ops)) + } + op := ops[0] + var claims []string + switch st.Kind() { + case statement.KindCreateTable: + if op.PartitionOf { + return statement.Statement{}, nil, ErrPartitionOfUnsupported + } + if op.Inherits { + return statement.Statement{}, nil, fmt.Errorf("%w: INHERITS binds to an existing parent the absence proof does not cover", ErrUnsupportedCreateStep) + } + if op.Like { + return statement.Statement{}, nil, fmt.Errorf("%w: LIKE reads an existing source table the absence proof does not cover", ErrUnsupportedCreateStep) + } + if op.OfType { + return statement.Statement{}, nil, fmt.Errorf("%w: OF binds to an existing composite type the absence proof does not cover", ErrUnsupportedCreateStep) + } + if op.IfNotExists { + return statement.Statement{}, nil, ErrIfNotExistsUnsupported + } + implicit, err := statement.ImplicitIndexNames(qualified) + if err != nil { + // ParseOne already admitted this SQL as a CREATE TABLE, so a + // refusal here means the two parse boundaries disagree. + return statement.Statement{}, nil, fmt.Errorf("%w: %w", ErrUnsupportedCreateStep, err) + } + claims = append([]string{st.Table()}, implicit...) + case statement.KindCreateIndex: + if op.Concurrent { + return statement.Statement{}, nil, fmt.Errorf("%w: a concurrent build is refused on a table born this run", ErrUnsupportedCreateStep) + } + if op.IfNotExists { + return statement.Statement{}, nil, ErrIfNotExistsUnsupported + } + if op.Name != "" { + claims = []string{op.Name} + } + default: + return statement.Statement{}, nil, fmt.Errorf("%w: kind %q", ErrUnsupportedCreateStep, st.Kind()) + } + // INV: ST-7 — the executor runs exactly the statement that was + // admitted, and only against the target the absence proof verified. + if st.Table() == "" || st.Schema() != at.Schema() || st.Table() != at.Table() { + return statement.Statement{}, nil, fmt.Errorf("%w: ST-7: statement targets %q but absence was verified for %q", + ErrInvariantViolation, qualifiedName(st.Schema(), st.Table()), qualifiedName(at.Schema(), at.Table())) + } + return st, claims, nil +} + +// asCreateCollision maps the duplicate-name SQLSTATEs — 42P07 when a +// relation holds the name, 42710 when a standalone type does — to the +// typed collision refusal. Every other error passes through unchanged. +// The server's error names the occupant, so the wrap adds the +// classification, not the identifier. +func asCreateCollision(err error) error { + var pgErr *pgconn.PgError + if !errors.As(err, &pgErr) { + return err + } + if pgErr.Code != sqlstateDuplicateTable && pgErr.Code != sqlstateDuplicateObject { + return err + } + return fmt.Errorf("%w: %w", ErrCreateCollision, err) +} diff --git a/pkg/executor/create_integration_test.go b/pkg/executor/create_integration_test.go new file mode 100644 index 0000000..7fbdb74 --- /dev/null +++ b/pkg/executor/create_integration_test.go @@ -0,0 +1,419 @@ +package executor_test + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" + "github.com/block/pg-sprite/pkg/dbconn" + "github.com/block/pg-sprite/pkg/executor" + "github.com/block/pg-sprite/pkg/preflight" + "github.com/block/pg-sprite/pkg/statement" +) + +// createFixture is one schema on a real server with an absence proof and +// a creation-privilege proof minted for the named table — the inputs +// ExecuteCreate requires. +type createFixture struct { + pool *pgxpool.Pool + schema string + at preflight.AbsentTarget + cr preflight.CreationRole +} + +func newCreateFixture(t *testing.T, table string) createFixture { + t.Helper() + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: testutil.StartPostgres(t)}) + require.NoError(t, err) + t.Cleanup(pool.Close) + schema := testutil.NewSchema(t, pool) + + at, err := preflight.CheckTableAbsent(t.Context(), pool, schema, table) + require.NoError(t, err) + cr, err := preflight.CheckCreatePrivileges(t.Context(), pool, schema) + require.NoError(t, err) + return createFixture{pool: pool, schema: schema, at: at, cr: cr} +} + +func desired(t *testing.T, sql string) statement.DesiredSchema { + t.Helper() + ds, err := statement.ParseDesired(sql) + require.NoError(t, err) + return ds +} + +// relationKind returns the pg_class relkind of schema.name, or "" when no +// relation owns the name — the catalog oracle for what a create run left. +func relationKind(t *testing.T, pool *pgxpool.Pool, schema, name string) string { + t.Helper() + var relkind *string + require.NoError(t, pool.QueryRow(t.Context(), + `SELECT c.relkind::text + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = $1 AND c.relname = $2`, + schema, name).Scan(&relkind)) + if relkind == nil { + return "" + } + return *relkind +} + +// relationExists reports whether any relation owns schema.name, for +// assertions where only presence matters. +func relationExists(t *testing.T, pool *pgxpool.Pool, schema, name string) bool { + t.Helper() + var exists bool + require.NoError(t, pool.QueryRow(t.Context(), + `SELECT EXISTS ( + SELECT FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = $1 AND c.relname = $2)`, + schema, name).Scan(&exists)) + return exists +} + +func TestExecuteCreateRunsTableAndIndexes(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, ` + CREATE TABLE t (id int PRIMARY KEY, name text); + CREATE INDEX t_name_idx ON t (name); + CREATE UNIQUE INDEX t_id_name_idx ON t (id, name); + `) + + rep, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.NoError(t, err) + + assert.Equal(t, "r", relationKind(t, f.pool, f.schema, "t")) + assert.Equal(t, "i", relationKind(t, f.pool, f.schema, "t_name_idx")) + assert.Equal(t, "i", relationKind(t, f.pool, f.schema, "t_id_name_idx")) + + require.Len(t, rep.Steps, 3) + assert.Contains(t, rep.Steps[0].SQL, "CREATE TABLE") + for _, step := range rep.Steps { + assert.Equal(t, executor.StepBrief, step.Kind) + assert.GreaterOrEqual(t, step.Duration, time.Duration(0)) + } +} + +// A desired file may state its index before its table — declarative input +// carries no ordering contract — but an index cannot be built before its +// table exists, so the executor orders the CREATE TABLE first. +func TestExecuteCreateOrdersTableBeforeIndexes(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, ` + CREATE INDEX t_name_idx ON t (name); + CREATE TABLE t (id int, name text); + `) + + rep, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.NoError(t, err) + + require.Len(t, rep.Steps, 2) + assert.Contains(t, rep.Steps[0].SQL, "CREATE TABLE") + assert.Equal(t, "i", relationKind(t, f.pool, f.schema, "t_name_idx")) +} + +// The absence proof is time-of-check: a create that takes the name after +// the check surfaces as the typed collision, and the caller re-diffs +// rather than assuming what the occupant looks like. +func TestExecuteCreateReportsCollisionAsTyped(t *testing.T) { + f := newCreateFixture(t, "t") + _, err := f.pool.Exec(t.Context(), fmt.Sprintf("CREATE TABLE %s.t (other int)", f.schema)) + require.NoError(t, err) + + ds := desired(t, "CREATE TABLE t (id int)") + rep, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.Error(t, err) + + var stepErr *executor.SequenceStepError + require.ErrorAs(t, err, &stepErr) + assert.Equal(t, 1, stepErr.Step) + assert.ErrorIs(t, err, executor.ErrCreateCollision) + assert.Equal(t, executor.CodeCreateCollision, executor.OutcomeCode(err)) + assert.Empty(t, rep.Steps) +} + +// A failed step ends the run; the steps before it committed and remain, +// and the report covers exactly that prefix so the caller can disclose +// what already happened. An index on a column the table does not have +// passes admission — admission checks shape and target, not column +// existence — and fails only when the server executes it. +func TestExecuteCreateFailedStepKeepsCommittedPrefix(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, ` + CREATE TABLE t (id int, name text); + CREATE INDEX t_id_idx ON t (id); + CREATE INDEX t_missing_idx ON t (missing); + `) + + rep, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.Error(t, err) + + var stepErr *executor.SequenceStepError + require.ErrorAs(t, err, &stepErr) + assert.Equal(t, 3, stepErr.Step) + assert.Equal(t, 3, stepErr.Total) + // The server error is not a collision; it passes through untyped. + assert.NotErrorIs(t, err, executor.ErrCreateCollision) + assert.Equal(t, executor.CodeExecutionFailed, executor.OutcomeCode(err)) + + assert.True(t, relationExists(t, f.pool, f.schema, "t")) + assert.True(t, relationExists(t, f.pool, f.schema, "t_id_idx")) + require.Len(t, rep.Steps, 2) + + // The committed prefix is the rerun contract: the absence check now + // refuses, which is the declarative front door's signal to re-diff. + _, err = preflight.CheckTableAbsent(t.Context(), f.pool, f.schema, "t") + assert.ErrorIs(t, err, preflight.ErrRelationExists) +} + +// A name claimed twice within the desired set is decidable at admission, +// so the whole set refuses before the first step runs — never a mid-run +// failure with a committed prefix. +func TestExecuteCreateRefusesDuplicateNamesAtAdmission(t *testing.T) { + tests := []struct { + name string + sql string + }{ + { + name: "two indexes under one name", + sql: `CREATE TABLE t (id int, name text); + CREATE INDEX dup_idx ON t (id); + CREATE INDEX dup_idx ON t (name)`, + }, + { + name: "index named after the table", + sql: `CREATE TABLE t (id int); + CREATE INDEX t ON t (id)`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, tt.sql) + + _, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrDuplicateCreateName) + assert.Equal(t, executor.CodeDuplicateCreateName, executor.OutcomeCode(err)) + assert.False(t, relationExists(t, f.pool, f.schema, "t"), + "admission covers the whole set before the first step executes") + }) + } +} + +// A standalone type occupying the table's name raises a different SQLSTATE +// than a relation would — every table also mints a composite type — and +// still surfaces as the typed collision. +func TestExecuteCreateReportsTypeCollisionAsTyped(t *testing.T) { + f := newCreateFixture(t, "t") + _, err := f.pool.Exec(t.Context(), fmt.Sprintf("CREATE TYPE %s.t AS ENUM ('a')", f.schema)) + require.NoError(t, err) + + ds := desired(t, "CREATE TABLE t (id int)") + _, err = executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrCreateCollision) + assert.Equal(t, executor.CodeCreateCollision, executor.OutcomeCode(err)) +} + +func TestExecuteCreateAdmissionRefusals(t *testing.T) { + tests := []struct { + name string + sql string + wantErr error + }{ + { + name: "if not exists on the table", + sql: "CREATE TABLE IF NOT EXISTS t (id int)", + wantErr: executor.ErrIfNotExistsUnsupported, + }, + { + name: "if not exists on an index", + sql: "CREATE TABLE t (id int); CREATE INDEX IF NOT EXISTS t_idx ON t (id)", + wantErr: executor.ErrIfNotExistsUnsupported, + }, + // INHERITS, LIKE, and OF bind to a secondary relation or type the + // qualification never touches: the name resolves via search_path + // to an existing object the absence proof says nothing about. + { + name: "inherits from an existing parent", + sql: "CREATE TABLE t (id int) INHERITS (parent)", + wantErr: executor.ErrUnsupportedCreateStep, + }, + { + name: "like an existing source table", + sql: "CREATE TABLE t (LIKE src INCLUDING ALL)", + wantErr: executor.ErrUnsupportedCreateStep, + }, + { + name: "of an existing composite type", + sql: "CREATE TABLE t OF ty", + wantErr: executor.ErrUnsupportedCreateStep, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, tt.sql) + + _, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, tt.wantErr) + assert.False(t, relationExists(t, f.pool, f.schema, "t"), + "admission covers the whole set before the first step executes") + }) + } +} + +func TestExecuteCreateRefusesPartitionOf(t *testing.T) { + f := newCreateFixture(t, "t_part") + _, err := f.pool.Exec(t.Context(), + fmt.Sprintf("CREATE TABLE %s.parent (id int) PARTITION BY RANGE (id)", f.schema)) + require.NoError(t, err) + + ds := desired(t, "CREATE TABLE t_part PARTITION OF parent FOR VALUES FROM (1) TO (10)") + _, err = executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrPartitionOfUnsupported) + assert.Equal(t, executor.CodePartitionOfUnsupported, executor.OutcomeCode(err)) +} + +// A desired schema for one table can never run against a proof minted for +// another: the mismatch is an invariant breach, not a refusal. +func TestExecuteCreateRefusesProofTargetMismatch(t *testing.T) { + f := newCreateFixture(t, "other") + ds := desired(t, "CREATE TABLE t (id int)") + + _, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrInvariantViolation) + assert.False(t, relationExists(t, f.pool, f.schema, "t")) +} + +// Create steps run with search_path pinned to the proof's schema then +// public — the same policy the introspection read path sets — so a +// desired file's unqualified type reference resolves in the target +// schema, and resolves there even when public holds a type of the same +// name. Without the pin the steps would run under the session default and +// the target schema's type would be invisible (SQLSTATE 42704). +func TestExecuteCreateResolvesTypesInTargetSchema(t *testing.T) { + f := newCreateFixture(t, "t") + + // The type lives in the target schema and, under a unique name, in + // public too — resolution must pick the target schema's copy. + typeName := f.schema + "_mood" + _, err := f.pool.Exec(t.Context(), fmt.Sprintf( + "CREATE TYPE %s.%s AS ENUM ('happy', 'sad')", f.schema, typeName)) + require.NoError(t, err) + _, err = f.pool.Exec(t.Context(), fmt.Sprintf( + "CREATE TYPE public.%s AS ENUM ('decoy')", typeName)) + require.NoError(t, err) + t.Cleanup(func() { + _, err := f.pool.Exec(context.WithoutCancel(t.Context()), + fmt.Sprintf("DROP TYPE IF EXISTS public.%s", typeName)) + assert.NoError(t, err) + }) + + ds := desired(t, fmt.Sprintf("CREATE TABLE t (id int, m %s)", typeName)) + _, err = executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.NoError(t, err) + + var udtSchema string + require.NoError(t, f.pool.QueryRow(t.Context(), + `SELECT udt_schema FROM information_schema.columns + WHERE table_schema = $1 AND table_name = 't' AND column_name = 'm'`, + f.schema).Scan(&udtSchema)) + assert.Equal(t, f.schema, udtSchema, + "the column's type must resolve in the proof's schema, not public") +} + +// An explicit CREATE INDEX whose name is the first choice of an implicit +// constraint index is a decidable conflict: admission refuses the whole +// set before anything runs, rather than letting the server suffix its way +// around one name or fail mid-run after the table committed. +func TestExecuteCreateRefusesImplicitIndexNameCollision(t *testing.T) { + tests := []struct { + name string + sql string + }{ + { + name: "explicit index named after the primary key's index", + sql: `CREATE TABLE t (id int PRIMARY KEY); + CREATE INDEX t_pkey ON t (id);`, + }, + { + name: "explicit index named after a unique constraint's index", + sql: `CREATE TABLE t (a int, b int, UNIQUE (a, b)); + CREATE INDEX t_a_b_key ON t (a);`, + }, + { + name: "explicit index named after a named constraint", + sql: `CREATE TABLE t (id int, CONSTRAINT my_uni UNIQUE (id)); + CREATE INDEX my_uni ON t (id);`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, tt.sql) + + _, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrDuplicateCreateName) + assert.Equal(t, executor.CodeDuplicateCreateName, executor.OutcomeCode(err)) + assert.False(t, relationExists(t, f.pool, f.schema, "t"), + "admission covers the whole set before the first step executes") + }) + } +} + +// A zero CreationRole is forgeable by any package: only +// CheckCreatePrivileges mints one with a schema, so the executor refuses +// it as an invariant breach before anything runs. +func TestExecuteCreateRefusesZeroCreationRole(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, "CREATE TABLE t (id int)") + + _, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, preflight.CreationRole{}, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrInvariantViolation) + assert.False(t, relationExists(t, f.pool, f.schema, "t")) +} + +// A creation-privilege proof minted for one schema can never authorize a +// run whose absence proof names another: the mismatch is an invariant +// breach, not a refusal. +func TestExecuteCreateRefusesCreationRoleSchemaMismatch(t *testing.T) { + f := newCreateFixture(t, "t") + otherSchema := testutil.NewSchema(t, f.pool) + otherCR, err := preflight.CheckCreatePrivileges(t.Context(), f.pool, otherSchema) + require.NoError(t, err) + + ds := desired(t, "CREATE TABLE t (id int)") + _, err = executor.ExecuteCreate(t.Context(), f.pool, f.at, otherCR, ds, createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrInvariantViolation) + assert.False(t, relationExists(t, f.pool, f.schema, "t")) +} + +// Unnamed CREATE INDEX steps claim no name — the server invents one, +// suffixing around occupants — so two of them in one desired set are not +// a duplicate-name conflict. +func TestExecuteCreateAllowsMultipleUnnamedIndexes(t *testing.T) { + f := newCreateFixture(t, "t") + ds := desired(t, ` + CREATE TABLE t (a int, b int); + CREATE INDEX ON t (a); + CREATE INDEX ON t (b); + `) + + rep, err := executor.ExecuteCreate(t.Context(), f.pool, f.at, f.cr, ds, createBudget, executor.DefaultRetryPolicy()) + require.NoError(t, err) + require.Len(t, rep.Steps, 3) + + var indexes int + require.NoError(t, f.pool.QueryRow(t.Context(), + `SELECT count(*) FROM pg_indexes WHERE schemaname = $1 AND tablename = 't'`, + f.schema).Scan(&indexes)) + assert.Equal(t, 2, indexes, "both server-named indexes exist") +} diff --git a/pkg/executor/create_test.go b/pkg/executor/create_test.go new file mode 100644 index 0000000..d126264 --- /dev/null +++ b/pkg/executor/create_test.go @@ -0,0 +1,59 @@ +package executor_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/executor" + "github.com/block/pg-sprite/pkg/preflight" + "github.com/block/pg-sprite/pkg/statement" +) + +// createBudget is generous for unit tests; admission refusals return +// before any database access. +var createBudget = executor.Budget{LockTimeout: time.Second, StatementTimeout: 2 * time.Second} + +func TestExecuteCreateRejectsUnboundedBudget(t *testing.T) { + ds, err := statement.ParseDesired("CREATE TABLE t (id int)") + require.NoError(t, err) + + _, err = executor.ExecuteCreate(t.Context(), nil, preflight.AbsentTarget{}, preflight.CreationRole{}, ds, + executor.Budget{LockTimeout: 0, StatementTimeout: time.Second}, executor.DefaultRetryPolicy()) + // The zero-value absence proof would also refuse (as an invariant + // violation); asserting on the budget wording proves the budget check + // fired first, since a valid proof needs a live database. + require.ErrorContains(t, err, "lock budget") +} + +// A zero-value AbsentTarget is constructible by any package; only +// CheckTableAbsent mints one with a verified target, so the executor +// refuses the forgery fail-closed. +func TestExecuteCreateRejectsZeroValueAbsenceProof(t *testing.T) { + ds, err := statement.ParseDesired("CREATE TABLE t (id int)") + require.NoError(t, err) + + _, err = executor.ExecuteCreate(t.Context(), nil, preflight.AbsentTarget{}, preflight.CreationRole{}, ds, + createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrInvariantViolation) +} + +// A zero-value DesiredSchema carries no admitted CREATE TABLE; only +// ParseDesired mints one, so the executor refuses the forgery fail-closed. +// The refusal fires even though the absence proof is also zero-valued: the +// absence check runs first and reports the same invariant class. +func TestExecuteCreateRejectsZeroValueDesiredSchema(t *testing.T) { + _, err := executor.ExecuteCreate(t.Context(), nil, preflight.AbsentTarget{}, preflight.CreationRole{}, statement.DesiredSchema{}, + createBudget, executor.DefaultRetryPolicy()) + require.ErrorIs(t, err, executor.ErrInvariantViolation) +} + +func TestExecuteCreateWithProgressRequiresTracker(t *testing.T) { + ds, err := statement.ParseDesired("CREATE TABLE t (id int)") + require.NoError(t, err) + + _, err = executor.ExecuteCreateWithProgress(t.Context(), nil, preflight.AbsentTarget{}, preflight.CreationRole{}, ds, + createBudget, executor.DefaultRetryPolicy(), nil) + require.ErrorIs(t, err, executor.ErrInvariantViolation) +} diff --git a/pkg/executor/docs_test.go b/pkg/executor/docs_test.go index 8c3bd13..2f359a5 100644 --- a/pkg/executor/docs_test.go +++ b/pkg/executor/docs_test.go @@ -25,3 +25,27 @@ func TestDocNamesEveryStepKind(t *testing.T) { "docs/execution-model.md does not name step kind %q", k) } } + +// Every outcome code automation can branch on must be named in the +// execution model: a Code added to the vocabulary without the doc naming +// it fails here. +func TestDocNamesEveryOutcomeCode(t *testing.T) { + raw, err := os.ReadFile(executionModelDoc) + require.NoError(t, err) + doc := string(raw) + for _, c := range executor.Codes() { + assert.Contains(t, doc, fmt.Sprintf("`%s`", c), + "docs/execution-model.md does not name outcome code %q", c) + } +} + +// The closed set has no duplicates: a code pasted twice would silently +// shadow a missing entry. +func TestCodesAreUnique(t *testing.T) { + seen := make(map[executor.Code]struct{}) + for _, c := range executor.Codes() { + _, dup := seen[c] + assert.False(t, dup, "duplicate outcome code %q", c) + seen[c] = struct{}{} + } +} diff --git a/pkg/executor/native.go b/pkg/executor/native.go index 357f164..782cef8 100644 --- a/pkg/executor/native.go +++ b/pkg/executor/native.go @@ -45,11 +45,12 @@ var ( // identical to the build session's — an unqualified name could resolve // to a different table and turn the verdict into a false clean. ErrUnqualifiedTable = errors.New("concurrent index build must schema-qualify its table") - // ErrIfNotExistsUnsupported is returned for CREATE INDEX CONCURRENTLY - // IF NOT EXISTS. The clause checks only the name: it succeeds as a - // no-op while an invalid or unrelated index owns that name, so the - // executor could report success over an index it cannot vouch for. - ErrIfNotExistsUnsupported = errors.New("CREATE INDEX CONCURRENTLY IF NOT EXISTS is not supported: a name-only no-op cannot prove the existing index is valid or even the requested one") + // ErrIfNotExistsUnsupported is returned for any CREATE ... IF NOT + // EXISTS. The clause checks only the name: it succeeds as a no-op + // while an unrelated relation — or, for a concurrent build, an + // invalid index — owns that name, so an executor could report + // success over a relation it cannot vouch for. + ErrIfNotExistsUnsupported = errors.New("IF NOT EXISTS is not supported: a name-only no-op cannot prove the existing relation is the requested one, or even valid") // ErrPreexistingInvalidIndex is returned when an invalid index with the // requested name already exists in the target schema — on any table. // The executor cannot prove who owns that entry — an in-progress diff --git a/pkg/executor/optimistic.go b/pkg/executor/optimistic.go index 575756a..94cb5f3 100644 --- a/pkg/executor/optimistic.go +++ b/pkg/executor/optimistic.go @@ -22,6 +22,7 @@ import ( "strconv" "time" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" @@ -218,6 +219,17 @@ func executeNative(ctx context.Context, pool *pgxpool.Pool, pt preflight.Preflig } func executeNativeAttempt(ctx context.Context, pool *pgxpool.Pool, st statement.Statement, b Budget) error { + return executeBoundedAttempt(ctx, pool, st, b, "") +} + +// executeBoundedAttempt is the shared transactional attempt behind the +// optimistic and create paths. When searchPathSchema is set, the +// transaction's search_path is pinned to that schema then public — the +// same policy the introspection read path sets — so a statement's +// unqualified references (a column's type, an expression's function) +// resolve exactly as the diff resolved them, never via the session's +// ambient search_path. +func executeBoundedAttempt(ctx context.Context, pool *pgxpool.Pool, st statement.Statement, b Budget, searchPathSchema string) error { tx, err := pool.Begin(ctx) if err != nil { return fmt.Errorf("begin optimistic attempt: %w", err) @@ -233,9 +245,12 @@ func executeNativeAttempt(ctx context.Context, pool *pgxpool.Pool, st statement. // INV: LK-2 — budgets are applied inside this transaction regardless of // the session defaults, so the attempt cannot outlive them even on a // misconfigured pool. A bare integer is milliseconds to PostgreSQL; - // SET LOCAL cannot use bind parameters. + // SET LOCAL cannot use bind parameters, and identifiers are sanitized. setBudgets := "SET LOCAL lock_timeout = " + strconv.FormatInt(b.LockTimeout.Milliseconds(), 10) + "; SET LOCAL statement_timeout = " + strconv.FormatInt(b.StatementTimeout.Milliseconds(), 10) + if searchPathSchema != "" { + setBudgets += "; SET LOCAL search_path = " + pgx.Identifier{searchPathSchema}.Sanitize() + ", public" + } if _, err := tx.Exec(ctx, setBudgets); err != nil { return fmt.Errorf("set attempt budgets: %w", err) } diff --git a/pkg/preflight/create.go b/pkg/preflight/create.go new file mode 100644 index 0000000..ab1f50c --- /dev/null +++ b/pkg/preflight/create.go @@ -0,0 +1,98 @@ +// This file is the create path's privilege check. A greenfield CREATE +// TABLE has no owner to be a member of — the table is born owned by the +// role that creates it — so the check proves the off-ladder TierCreateTable +// facts (CONNECT on the database, USAGE and CREATE on the schema) instead +// of walking the ownership tier ladder in privileges.go, which states +// facts about an existing table. + +package preflight + +import ( + "context" + "fmt" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" +) + +// CreationRole proves the connected role holds every access a greenfield +// CREATE TABLE in the schema needs: CONNECT on the database, USAGE and +// CREATE on the schema. It can only be constructed by +// CheckCreatePrivileges in this package. The schema it carries is always +// resolved — an unqualified check records the session's creation schema. +// +// Like AbsentTarget, the proof is time-of-check and session-scoped: a +// grant can be revoked between the check and the CREATE TABLE, in which +// case the create fails with the server's own insufficient-privilege +// error rather than a typed refusal. +type CreationRole struct { + role string + schema string +} + +// Role returns the connected role the checks ran as — the role a created +// table would be owned by. +func (c CreationRole) Role() string { return c.role } + +// Schema returns the resolved schema the access was verified in. +func (c CreationRole) Schema() string { return c.schema } + +// CheckCreatePrivileges verifies the connected role can create a table in +// the schema (the session's creation schema, current_schema(), when schema +// is empty): CONNECT on the database, USAGE and CREATE on the schema. A +// missing grant is a *PrivilegeError naming the exact statement that would +// satisfy it — the grantee is the connected role itself, because a table +// that does not exist yet has no owning role to inherit from. On success +// it returns the CreationRole proof. +func CheckCreatePrivileges(ctx context.Context, pool *pgxpool.Pool, schema string) (CreationRole, error) { + // One catalog snapshot gathers every fact the check consults, so the + // facts cannot disagree about when they looked. The LEFT JOIN turns + // "schema missing" into a false exists column instead of an absent + // row, and COALESCE keeps the privilege probes NULL-safe on that + // branch. + const q = ` + SELECT s.nspname, + n.nspname IS NOT NULL, + current_user::text, + current_database()::text, + has_database_privilege(current_user, current_database(), 'CONNECT'), + COALESCE(has_schema_privilege(current_user, n.nspname, 'USAGE'), false), + COALESCE(has_schema_privilege(current_user, n.nspname, 'CREATE'), false) + FROM (SELECT CASE WHEN $1 = '' THEN current_schema() ELSE $1 END AS nspname) s + LEFT JOIN pg_namespace n ON n.nspname = s.nspname` + var targetSchema *string + var schemaExists, canConnect, schemaUsage, schemaCreate bool + var role, database string + if err := pool.QueryRow(ctx, q, schema).Scan( + &targetSchema, &schemaExists, &role, &database, + &canConnect, &schemaUsage, &schemaCreate); err != nil { + return CreationRole{}, fmt.Errorf("gather create access facts for schema %q: %w", schema, err) + } + if targetSchema == nil { + // Only an unqualified check can land here: current_schema() is + // NULL when the search_path names no usable schema, so there is + // no schema to check creation access in. + return CreationRole{}, fmt.Errorf("resolve creation schema: %w", ErrNoCreationSchema) + } + if !schemaExists { + return CreationRole{}, fmt.Errorf("%w: schema %s does not exist", ErrSchemaNotFound, *targetSchema) + } + // INV: ST-6 — each missing grant is a typed refusal carrying the exact + // provisioning statement; the proof is only minted when every fact + // holds. + if !canConnect { + return CreationRole{}, connectRefusal(role, database) + } + if !schemaUsage { + return CreationRole{}, schemaUsageRefusal(role, *targetSchema) + } + if !schemaCreate { + return CreationRole{}, &PrivilegeError{ + Tier: TierCreateTable, + Check: fmt.Sprintf("has_schema_privilege(%s, %s, 'CREATE')", role, *targetSchema), + Grant: fmt.Sprintf("GRANT CREATE ON SCHEMA %s TO %s", + pgx.Identifier{*targetSchema}.Sanitize(), pgx.Identifier{role}.Sanitize()), + } + } + return CreationRole{role: role, schema: *targetSchema}, nil +} diff --git a/pkg/preflight/create_integration_test.go b/pkg/preflight/create_integration_test.go new file mode 100644 index 0000000..3cb81be --- /dev/null +++ b/pkg/preflight/create_integration_test.go @@ -0,0 +1,104 @@ +package preflight_test + +import ( + "fmt" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" + "github.com/block/pg-sprite/pkg/dbconn" + "github.com/block/pg-sprite/pkg/preflight" +) + +// The off-ladder create check, walked grant by grant: each missing access +// is a typed refusal naming the exact provisioning statement whose grantee +// is the engine role itself, and applying exactly that statement unlocks +// the next rung. +func TestCheckCreatePrivilegesWalksTheGrants(t *testing.T) { + serverURL := testutil.StartPostgres(t) + admin, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: serverURL}) + require.NoError(t, err) + t.Cleanup(admin.Close) + schema := testutil.NewSchema(t, admin) + + const password = "create-test-password" + role := testutil.NewRole(t, admin, "LOGIN PASSWORD '"+password+"'") + engine := connectAs(t, serverURL, role, password) + ctx := t.Context() + + // No USAGE on the schema yet. + _, err = preflight.CheckCreatePrivileges(ctx, engine, schema) + var privErr *preflight.PrivilegeError + require.ErrorAs(t, err, &privErr) + assert.Equal(t, preflight.TierConnect, privErr.Tier) + assert.Equal(t, fmt.Sprintf("GRANT USAGE ON SCHEMA %s TO %s", + pgx.Identifier{schema}.Sanitize(), pgx.Identifier{role}.Sanitize()), privErr.Grant) + + _, err = admin.Exec(ctx, privErr.Grant) + require.NoError(t, err) + + // USAGE held, CREATE still missing: the off-ladder tier, and the + // grantee is the connected role — no owner exists to inherit from. + _, err = preflight.CheckCreatePrivileges(ctx, engine, schema) + require.ErrorAs(t, err, &privErr) + assert.Equal(t, preflight.TierCreateTable, privErr.Tier) + assert.Equal(t, fmt.Sprintf("GRANT CREATE ON SCHEMA %s TO %s", + pgx.Identifier{schema}.Sanitize(), pgx.Identifier{role}.Sanitize()), privErr.Grant) + assert.Empty(t, privErr.Hint, "the grant speaks for itself: its grantee is the role the check names") + + _, err = admin.Exec(ctx, privErr.Grant) + require.NoError(t, err) + + proof, err := preflight.CheckCreatePrivileges(ctx, engine, schema) + require.NoError(t, err) + assert.Equal(t, role, proof.Role()) + assert.Equal(t, schema, proof.Schema()) +} + +func TestCheckCreatePrivilegesRefusesMissingSchema(t *testing.T) { + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: testutil.StartPostgres(t)}) + require.NoError(t, err) + t.Cleanup(pool.Close) + + _, err = preflight.CheckCreatePrivileges(t.Context(), pool, "no_such_schema") + assert.ErrorIs(t, err, preflight.ErrSchemaNotFound) +} + +// An empty schema resolves the session's creation schema — the schema an +// unqualified CREATE TABLE would land in — and the proof carries it. +func TestCheckCreatePrivilegesResolvesUnqualifiedSchema(t *testing.T) { + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: testutil.StartPostgres(t)}) + require.NoError(t, err) + t.Cleanup(pool.Close) + + var creationSchema string + require.NoError(t, pool.QueryRow(t.Context(), "SELECT current_schema()").Scan(&creationSchema)) + + proof, err := preflight.CheckCreatePrivileges(t.Context(), pool, "") + require.NoError(t, err) + assert.Equal(t, creationSchema, proof.Schema()) +} + +// A session whose search_path names no schema has no creation target for +// an unqualified check; the check fails rather than guessing a schema. +func TestCheckCreatePrivilegesRefusesEmptySearchPath(t *testing.T) { + serverURL := testutil.StartPostgres(t) + admin, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: serverURL}) + require.NoError(t, err) + t.Cleanup(admin.Close) + + const password = "create-test-password" + role := testutil.NewRole(t, admin, "LOGIN PASSWORD '"+password+"'") + _, err = admin.Exec(t.Context(), fmt.Sprintf("ALTER ROLE %s SET search_path = ''", + pgx.Identifier{role}.Sanitize())) + require.NoError(t, err) + + engine := connectAs(t, serverURL, role, password) + _, err = preflight.CheckCreatePrivileges(t.Context(), engine, "") + assert.ErrorIs(t, err, preflight.ErrNoCreationSchema) + assert.NotErrorIs(t, err, preflight.ErrSchemaNotFound, + "an unresolvable search_path is not a missing schema — there is no schema name to report missing") +} diff --git a/pkg/preflight/docs_test.go b/pkg/preflight/docs_test.go index b676067..e5c7e54 100644 --- a/pkg/preflight/docs_test.go +++ b/pkg/preflight/docs_test.go @@ -22,7 +22,7 @@ var proofTypeDocs = []string{ // document: a new proof type added without updating all three lists fails // here. Extend the slice when a new proof type lands. func TestDocsListEveryProofType(t *testing.T) { - proofTypes := []string{"PreflightedTable", "AbsentTarget"} + proofTypes := []string{"PreflightedTable", "AbsentTarget", "CreationRole"} for _, doc := range proofTypeDocs { raw, err := os.ReadFile(doc) require.NoError(t, err) diff --git a/pkg/preflight/privileges.go b/pkg/preflight/privileges.go index 68284ef..3ac8583 100644 --- a/pkg/preflight/privileges.go +++ b/pkg/preflight/privileges.go @@ -40,6 +40,14 @@ const ( // TierCopyAndSwap covers shadow-object creation: membership usable // with SET ROLE, so shadow objects are born with the correct owner. TierCopyAndSwap + // TierCreateTable covers greenfield CREATE TABLE and sits off the + // ladder above: a table that does not exist yet has no owner to be a + // member of, so the create path proves CONNECT on the database plus + // USAGE and CREATE on the schema — deliberately not the ownership + // membership the ALTER tiers require. It is checked by + // CheckCreatePrivileges, never by CheckPrivileges, whose ladder walks + // facts about an existing table. + TierCreateTable ) // String names the tier's capability for refusal messages. @@ -53,6 +61,8 @@ func (t Tier) String() string { return "index builds" case TierCopyAndSwap: return "copy-and-swap" + case TierCreateTable: + return "create a new table" default: return fmt.Sprintf("unknown tier %d", int(t)) } @@ -249,16 +259,33 @@ func unresolvedTargetCause(ctx context.Context, pool *pgxpool.Pool, schema, tabl return fmt.Errorf("resolve schema %s: %w", schema, err) } if !usage { - return &PrivilegeError{ - Tier: TierConnect, - Check: fmt.Sprintf("has_schema_privilege(%s, %s, 'USAGE')", role, schema), - Grant: fmt.Sprintf("GRANT USAGE ON SCHEMA %s TO %s", - pgx.Identifier{schema}.Sanitize(), pgx.Identifier{role}.Sanitize()), - } + return schemaUsageRefusal(role, schema) } return fmt.Errorf("%w: %s", ErrTableNotFound, qualifiedName(schema, table)) } +// connectRefusal is the typed refusal for a role that cannot connect to +// the database, carrying the exact provisioning statement. +func connectRefusal(role, database string) *PrivilegeError { + return &PrivilegeError{ + Tier: TierConnect, + Check: fmt.Sprintf("has_database_privilege(%s, %s, 'CONNECT')", role, database), + Grant: fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s", + pgx.Identifier{database}.Sanitize(), pgx.Identifier{role}.Sanitize()), + } +} + +// schemaUsageRefusal is the typed refusal for a role without USAGE on the +// schema, carrying the exact provisioning statement. +func schemaUsageRefusal(role, schema string) *PrivilegeError { + return &PrivilegeError{ + Tier: TierConnect, + Check: fmt.Sprintf("has_schema_privilege(%s, %s, 'USAGE')", role, schema), + Grant: fmt.Sprintf("GRANT USAGE ON SCHEMA %s TO %s", + pgx.Identifier{schema}.Sanitize(), pgx.Identifier{role}.Sanitize()), + } +} + // checkTierLadder walks the contract's tiers bottom-up to the requirement // and returns the first missing access as a typed refusal. Bottom-up order // makes the refusal actionable: the operator fixes the foundational grant @@ -270,20 +297,10 @@ func unresolvedTargetCause(ctx context.Context, pool *pgxpool.Pool, schema, tabl // mid-change server error. func checkTierLadder(ctx context.Context, pool *pgxpool.Pool, f accessFacts, tier Tier) error { if !f.canConnect { - return &PrivilegeError{ - Tier: TierConnect, - Check: fmt.Sprintf("has_database_privilege(%s, %s, 'CONNECT')", f.role, f.database), - Grant: fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s", - pgx.Identifier{f.database}.Sanitize(), pgx.Identifier{f.role}.Sanitize()), - } + return connectRefusal(f.role, f.database) } if !f.schemaUsage { - return &PrivilegeError{ - Tier: TierConnect, - Check: fmt.Sprintf("has_schema_privilege(%s, %s, 'USAGE')", f.role, f.schema), - Grant: fmt.Sprintf("GRANT USAGE ON SCHEMA %s TO %s", - pgx.Identifier{f.schema}.Sanitize(), pgx.Identifier{f.role}.Sanitize()), - } + return schemaUsageRefusal(f.role, f.schema) } if tier < TierAlterInPlace { return nil diff --git a/pkg/statement/implicit.go b/pkg/statement/implicit.go new file mode 100644 index 0000000..0957923 --- /dev/null +++ b/pkg/statement/implicit.go @@ -0,0 +1,180 @@ +// This file predicts the index names PostgreSQL invents for a CREATE +// TABLE's index-backed constraints. The create path's admission gate +// claims every relation name a desired set will occupy, and an implicit +// constraint index occupies one just as an explicit CREATE INDEX does — +// a set whose explicit index name collides with a constraint's index +// would otherwise pass admission and fail mid-run after the table +// committed. The prediction mirrors the server's first choice +// (makeObjectName in the PostgreSQL sources): when that first choice is +// already occupied on the server, PostgreSQL appends a numeric suffix +// instead, so a predicted name is where the server *starts*, not a +// guarantee of the final catalog name — exactly the right meaning for a +// duplicate-claim check inside one desired set. + +package statement + +import ( + "errors" + "fmt" + "strings" + "unicode/utf8" + + pganalyze "github.com/pganalyze/pg_query_go/v6" + pgquery "github.com/wasilibs/go-pgquery" +) + +// ErrNotCreateTable is returned when the statement handed to +// ImplicitIndexNames is not a single CREATE TABLE. +var ErrNotCreateTable = errors.New("statement is not a CREATE TABLE") + +// nameDataLen is PostgreSQL's NAMEDATALEN - 1: the byte budget an +// identifier is truncated to. +const nameDataLen = 63 + +// ImplicitIndexNames returns the first-choice index names PostgreSQL will +// use for the index-backed constraints of one CREATE TABLE statement — +// PRIMARY KEY, UNIQUE, and EXCLUDE, in their column-inline and +// table-constraint forms. A named constraint's index takes the constraint +// name verbatim; an unnamed one takes the server's generated name +// (`