feat(executor): prove claimed relation names free before the create runs

[Kiran01bm]

The create path now probes pg_class for every relation name the desired set states — explicit CREATE INDEX names, the first-choice name of each PRIMARY KEY/UNIQUE/EXCLUDE constraint index, and the <table>_<column>_seq sequence each serial or identity column owns — before the first step runs, refusing an occupied name as a typed create-collision with nothing executed.

Why

CheckTableAbsent proved only the table name free, and the other names a CREATE TABLE occupies behaved differently when an unrelated relation already held one:

• Explicit CREATE INDEX t_v_idx ON t (v): the CREATE TABLE committed, then the index step failed with a duplicate-name SQLSTATE. The rerun saw a table that exists with a shape the desired file does not state, and the reviewer got a mid-run failure rather than a refusal on a clean catalog.
• Implicit PRIMARY KEY / UNIQUE index, serial / GENERATED AS IDENTITY sequence: the create succeeded — PostgreSQL sidesteps the occupied first-choice name with a numeric suffix (t_pkey1, t_id_seq1) inside the CREATE TABLE. The catalog then holds an object whose name is not the one the desired file implies, and a later re-diff of the same file cannot tell it apart from a stray. An adopter's first desired file is very often id serial PRIMARY KEY, so the sequence case is the one they hit first.

This PR treats the first-choice name as part of the desired file's contract: every case now refuses before anything runs. For the implicit cases that is a policy change, not a bug fix — a desired file that used to converge with a suffixed index or sequence name is now refused until the occupant is dropped or renamed, the constraint's index is named explicitly, or the column uses an explicitly named sequence or a non-serial type.

What

• statement.ImplicitRelationNames(sql) returns the first-choice relation names one CREATE TABLE will occupy: constraint-index names (named constraints verbatim, unnamed ones via the server's generation and 63-byte truncation) plus <table>_<column>_seq for serial/smallserial/bigserial and identity columns, under the same truncation rules.
• preflight.CheckNamesAbsent(ctx, pool, at AbsentTarget, names): one schema-scoped pg_class probe over the claimed names, wrapping ErrRelationExists. The schema comes from the absence proof, so an empty or unresolved schema is an error rather than an all-clear. It deliberately does not probe pg_type — index and sequence names create no composite types; the table's is already covered by CheckTableAbsent. ORDER BY relname decides which occupant is reported.
• executeCreate runs the probe over the claimed names (minus the table) before the step loop. Only preflight.IsNameOccupied hits become ErrCreateCollision; a probe that itself fails (cancelled context, dropped connection) is returned as an operational error, not a collision.
• migrate.runCreate maps a pre-execution ErrCreateCollision to a refused verdict with create-collision whose detail names the occupant and the remedies, so callers see a refusal, not a failed apply.
• Coverage boundary: names the server invents are not claimed — an unnamed CREATE INDEX ON t (v) claims nothing. Duplicate-name SQLSTATEs backstop time-of-check races for explicit names; for server-chosen names the probe narrows the race to the time-of-check window, and nothing catches a name taken inside it. Docs state both boundaries plainly, and the SchemaBot integration guide notes that a create-collision may be about a name the table needs (index, constraint index, sequence) rather than the table itself.

Before / after

Before                                        After
explicit CREATE INDEX t_v_idx, t_v_idx taken  CheckTableAbsent(t)              ok
  CREATE TABLE t        committed             CheckCreatePrivileges            ok
  CREATE INDEX t_v_idx  42P07 -> failed       CheckNamesAbsent(at, [t_pkey, t_id_seq, t_v_idx])
  table t left behind, rerun re-diffs           -> refused create-collision
                                                 nothing executed, catalog unchanged
implicit PRIMARY KEY, t_pkey taken              detail: drop/rename the occupant,
  CREATE TABLE t        committed as t_pkey1     name the constraint's index, or use
id serial, t_id_seq taken                        an explicitly named sequence /
  CREATE TABLE t        committed as t_id_seq1   non-serial column
  desired file's implied names never exist

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 Adversarial correctness review, requested by Armand and performed by his agent. Reviewed at head 2fc6a04c.

Verdict: the probe is correct, fail-closed on its own failure, and the right shape — nothing blocks. I attacked the claim "every name the desired file states is proved free", and it holds for indexes and constraint indexes: ImplicitIndexNames covers table-level and inline PRIMARY KEY/UNIQUE, EXCLUDE, named constraints, and the server's expr substitution. Three findings, all proved against a real PostgreSQL 16: one unenforced precondition in a core-package export, and two places where the stated boundary is narrower than the real one.

Findings

1. CheckNamesAbsent reports every name free when schema is empty or does not exist, and nothing enforces otherwise. The godoc states the precondition ("must be the resolved, non-empty schema … the one an AbsentTarget carries") but the body only filters n.nspname = $1, so a wrong schema yields pgx.ErrNoRows → nil → all-clear. Proved: with <schema>.occupied existing, CheckNamesAbsent(ctx, pool, "", []string{"occupied"}) returns nil, and so does "no_such_schema". Today's only caller passes at.Schema() so it can't trip, but this is an exported function in a core package whose failure mode is a safety check that passes, and SAFETY.md is explicit on both halves — "never trust callers … the core enforces", and dangerous APIs take proof types, "never a raw string … a caller could fabricate." The proof type already exists and the caller already holds it: taking preflight.AbsentTarget instead of schema string makes the illegal state unrepresentable and costs one signature line.

2. "Duplicate-name SQLSTATEs remain the race backstop" is true for explicit index names and false for exactly the case that motivated this PR. The implicit constraint index does not raise 42P07/42710 — PostgreSQL picks a non-conflicting name inside the CREATE TABLE, which is the silent-suffix behavior the Why section documents. Proved: with t_pkey occupied, CREATE TABLE t (id int PRIMARY KEY) succeeds and the resulting index is t_pkey1. So in the time-of-check window the implicit case has no backstop: the probe passes, another session takes t_pkey, the create succeeds, and the desired file's contract is violated silently — narrowed to a window, not closed. The claim appears in the create.go header, the ErrCreateCollision godoc, and four docs pages. Detection is cheap and unambiguous precisely because absence was proved first: after the CREATE TABLE step, the actual constraint-index names either match the claimed first-choice names or the race happened.

3. serial / GENERATED AS IDENTITY sequence names are part of the desired file's contract, are not in the claim set, and do not fail closed. ImplicitIndexNames returns index names only — by name and by design — so CREATE TABLE s (id serial PRIMARY KEY) claims [s_pkey] and never s_id_seq. Proved for both spellings: with s_id_seq occupied, the create succeeds and the sequence lands as s_id_seq1; identical for id bigint GENERATED BY DEFAULT AS IDENTITY. This is the same defect class the PR closes for constraint indexes — the catalog holds an object whose name the desired file implies but does not have, and a later re-diff cannot tell it from a stray — except it needs no race at all: it happens on a quiet catalog, every time. The machinery is already in place, since sequences live in pg_class and the probe does not filter relkind; what's missing is the name in the claim set.

Action items

1. (Finding 1) Change CheckNamesAbsent to accept preflight.AbsentTarget rather than schema string, deriving the schema from the proof. If the raw-string signature has a caller you want to keep, reject schema == "" and add the case to TestCheckNamesAbsent.
2. (Finding 3) Add the column-owned sequence names (<table>_<column>_seq for serial and identity columns) to the claim set, and cover both spellings in create_integration_test.go. Sequences are already in scope for the probe's query.
3. (Finding 2) Qualify the backstop sentence wherever it appears: the duplicate-name SQLSTATEs cover explicit index names, and the implicit constraint index has no SQLSTATE to catch. Either state the residual window plainly, or close it by comparing the constraint indexes' real names against the claimed ones after the CREATE TABLE step.
4. (optional) docs/schemabot-integration.md names the unnamed CREATE INDEX as the coverage boundary; once finding 3 lands, the boundary is worth restating as "names the server invents", which is the property that actually decides it.

Verified (tried to break, couldn't)

ImplicitIndexNames covers table-level and inline PRIMARY KEY/UNIQUE with their different first-choice forms (t_pkey vs t_col_key), EXCLUDE (_excl), explicitly named constraints via conname, and the literal expr the server substitutes for expression elements; FOREIGN KEY/CHECK build no index and are correctly absent; the probe deliberately omits a relkind filter, which is right because relation names share one per-schema namespace — a table squatting an index name is caught, and the new test pins cross-schema isolation; parameters carry the names so nothing is interpolated; IsNameOccupied gates the collision mapping and any other probe error returns wrapped as operational, so an unreachable catalog cannot become a passing check or a false refusal; slices.DeleteFunc mutates a slice freshly built from the local map, so there is no aliasing, and the map's non-deterministic order is neutralized by ORDER BY relname LIMIT 1 exactly as the comment claims; the probe is one bounded query under dbconn's statement timeout with the name list bounded by the desired set; INV ST-7/ST-8 checks and the PARTITION OF/INHERITS/LIKE/OF/IF NOT EXISTS refusals are unchanged; the privilege-vs-index-collision ordering change is real and is documented honestly rather than papered over; go build ./... clean and ./pkg/preflight ./pkg/executor ./pkg/migrate ./pkg/statement all pass locally at head; 14/14 CI green; no tests or assertions removed.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Second pass on the same head (2fc6a04c), through the two lenses that matter beyond correctness: what an outside adopter experiences, and what SchemaBot does with the new refusal.

Lens 1 — outside adopter

Strong. The refusal text in migrate/desired.go is the best kind: it names the occupant, states that nothing was executed, and gives two concrete ways out (drop or rename the occupant, or name the constraint's index explicitly). That is a refusal an operator can act on without reading the source. docs/limitations.md and docs/capabilities.md both land the "proved free before, not discovered during" framing well, and cli-output-examples.md showing the actual rendered refusal is exactly the right way to document it.

Two rough edges an adopter will hit before we do:

1. The sequence gap in finding 3 of the correctness pass is the one place a first run leaves a silently renamed object behind. An adopter's first desired file is very often id serial PRIMARY KEY, and if <table>_id_seq happens to be taken, they get a successful create and a sequence they did not name — the exact surprise this PR exists to remove, with none of its subtlety.
2. docs/limitations.md reads as if the only uncovered case is an unnamed CREATE INDEX ON t (v). Once the boundary is stated as "names the server invents rather than names you wrote", both the index case and the sequence case fall out of one sentence, and the doc stops needing an enumeration that will drift.

Lens 2 — SchemaBot integration

The wire contract is unchanged (ErrCreateCollision and CodeCreateCollision both already existed), so nothing breaks on bump — but the conditions under which SchemaBot renders those two branches change materially, and its text is now wrong in two ways. pkg/engine/postgres/apply.go handles both:

• The preflight.IsNameOccupied branch says a relation already occupies the name %q on the target with the table name interpolated. After this PR the occupied name is usually an index or constraint name, not the table — the table's own absence was proved separately, one step earlier. The message will name an object that is demonstrably free.
• Both that branch and the CodeCreateCollision branch tell the operator to re-plan against the current schema. This PR's own docs are explicit that re-planning does not clear a name collision: the desired file still claims the name, so the next plan produces the same refusal. The remedy is the one pg-sprite already wrote — drop or rename the occupant, or name the constraint's index explicitly.

Both sites synthesize their own detail and discard pg-sprite's Detail, which is why the drift is invisible: refusalForOutcome's exhaustiveness test over executor.Codes() still passes, because the vocabulary didn't move — only the firing conditions and the correct advice did. That's a SchemaBot-side follow-up, not a blocker on this PR; I'll carry it. Worth a line in docs/schemabot-integration.md noting that a create-collision may now be about a name the table needs rather than the table itself, so the next consumer doesn't inherit the same wrong assumption.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Approving — the probe is correct and fail-closed, and my three findings are all follow-ups rather than blockers: an unenforced precondition on a core-package export, one uncovered claimed-name class (sequences), and a backstop claim in the docs that is narrower than stated. Details in the two review comments above.

This review was generated by Claude Code (claude-opus-5).

[Kiran01bm]

🤖 Review response — created by Kiran's code review agent (Amp, Claude Opus 4.5) — pull/71, follow-up commit

Source: review comment 5103477739 (adversarial pass), 5103478419 (second pass), 5103479585 (approval) at head 2fc6a04. Fixes are in the follow-up commit on this branch.

#	Concern	Status
3	serial / GENERATED AS IDENTITY sequence names (<table>_<column>_seq) are part of the desired file's contract, were not in the claim set, and did not fail closed — the create succeeded with s_id_seq1	fixed — ImplicitIndexNames is now ImplicitRelationNames and adds the column-owned sequence name for serial/smallserial/bigserial and identity columns via the existing makeObjectName(table, column, "seq") (same 63-byte truncation the server applies); columnOwnsSequence helper; integration tests cover both spellings with an occupant sequence → ErrCreateCollision, nothing executed. Refusal text now also offers "for a sequence use an explicitly named sequence or a non-serial column"
1	CheckNamesAbsent reported every name free when schema was empty or nonexistent; the godoc precondition was unenforced on a core-package export	fixed — signature is CheckNamesAbsent(ctx, pool, at AbsentTarget, names), deriving the schema from the absence proof; returns an error when the proof carries no verified target; TestCheckNamesAbsent covers it
2	"Duplicate-name SQLSTATEs remain the race backstop" is true for explicit names and false for server-chosen ones — the implicit index (and sequence) gets a suffix instead of 42P07/42710	fixed (docs + comments) — every occurrence now reads: SQLSTATEs backstop races for explicit names; for server-chosen names the probe narrows the race to the time-of-check window and nothing catches a name taken inside it. Post-create verification of the real relation names against the claimed ones was not implemented in this PR — deferred, tracked as an internal follow-up
optional	docs/schemabot-integration.md and docs/limitations.md named the unnamed CREATE INDEX as the coverage boundary; the property that decides it is "names the server invents"	fixed — boundary restated as server-invented names (covers the unnamed index and the sequence case in one sentence); schemabot-integration.md notes a create-collision may now be about a name the table needs (index, constraint index, sequence) rather than the table itself
second pass	SchemaBot's pkg/engine/postgres/apply.go interpolates the table name into the IsNameOccupied message and tells the operator to re-plan, which does not clear a name collision	reply — SchemaBot-side follow-up (the reviewer is carrying it); this PR adds the integration-doc line so the next consumer does not inherit the assumption

Verified: gofmt -l pkg/ internal/ clean; go build ./... && go vet ./... && go vet -tags=integration ./...; golangci-lint cache clean && make lint 0 issues; make test-unit; go test -tags=integration -count=1 ./pkg/... ./internal/cli/ against compose Postgres; make demo-check.