Skip to content

executor: ExecuteCreate — the create path for a verified-absent target - #62

Merged
Kiran01bm merged 3 commits into
mainfrom
kiran01bm/ct2-executor-create-path
Aug 28, 2026
Merged

executor: ExecuteCreate — the create path for a verified-absent target#62
Kiran01bm merged 3 commits into
mainfrom
kiran01bm/ct2-executor-create-path

Conversation

@Kiran01bm

@Kiran01bm Kiran01bm commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Adds the create-path executor: ExecuteCreate runs a validated desired schema (one CREATE TABLE plus its indexes) against a name proven absent, with an off-ladder privilege proof for greenfield creation.

Why

The declarative front door can diff a desired table into existence, but nothing below it could execute that creation under the engine's proof discipline: the sequence executor consumes a PreflightedTable, which by definition cannot exist for a table that does not. The create path needs its own proof pair — the target name is free (AbsentTarget, already landed) and the role may create in the schema — and an executor that re-verifies both at the point of use. This lands that executor, dormant until the front door routes to it.

What

  • executor.ExecuteCreate / ExecuteCreateWithProgress: qualifies every desired statement into the proof's schema, re-parses and admits by shape and target (ST-7), orders the CREATE TABLE first, and runs each step as a brief bounded transaction under the existing lock-retry machinery. Failure returns the committed-prefix SequenceReport contract; the duplicate-name SQLSTATEs (42P07 for a relation, 42710 for a standalone type holding the name) map to the typed ErrCreateCollision so the caller re-diffs instead of assuming.
  • Indexes build plainly, never CONCURRENTLY: the table is born this run with no traffic to protect, a plain build on an empty table is fast, and it cannot leave an INVALID index behind a failure.
  • Refusals, all at admission before anything executes: IF NOT EXISTS (table or index — a name-only no-op proves nothing); CREATE TABLE PARTITION OF, INHERITS, LIKE, and OF type (each binds a secondary relation or type the qualification never touches, so the name resolves via search_path to an existing object the absence proof does not cover); concurrent index builds; and a name claimed twice within the desired set (ErrDuplicateCreateName — decidable at admission, never a mid-run failure with a committed prefix).
  • preflight.CheckCreatePrivilegesCreationRole proof: one catalog snapshot proving CONNECT + schema USAGE + CREATE, with each missing grant a typed *PrivilegeError whose grantee is the engine role itself. Off the ownership tier ladder deliberately — a greenfield table has no owner to be a member of; it is born owned by its creator (TierCreateTable).
  • statement.Op now carries IfNotExists, Inherits, Like, and OfType for CREATE TABLE; four new outcome codes (create-collision, duplicate-create-name, partition-of-unsupported, unsupported-create-step); docs updated (ST-7 enforcement list, SAFETY.md / tcb-model.md / review-checks proof types, engine-role.md off-ladder section, capabilities/limitations/README create-path boundaries).

Before / after

Before: no execution path for a desired table that does not exist yet

  ParseDesired ──▶ DesiredSchema ──▶ (no executor consumes it)
  CheckTableAbsent ──▶ AbsentTarget ──▶ (no executor consumes it)

After: the create path, proof-gated end to end

  CheckCreatePrivileges ──▶ CreationRole        (may I create here?)
  CheckTableAbsent ─────────▶ AbsentTarget      (is the name free?)
                                   │
  ParseDesired ──▶ DesiredSchema ──┤
                                   ▼
                            ExecuteCreate
                    qualify + re-parse + admit (ST-7)
          refuse: PARTITION OF / INHERITS / LIKE / OF /
                  IF NOT EXISTS / CONCURRENTLY / duplicate names
                                   │
                    ┌──────────────┼──────────────┐
                    ▼              ▼              ▼
              CREATE TABLE   CREATE INDEX   CREATE INDEX ...
               (always 1st)  (input order, plain builds, brief budgets)

  42P07 / 42710 ──▶ ErrCreateCollision ──▶ caller re-diffs the live catalog
  failed step ──▶ committed prefix remains ──▶ rerun refuses ErrRelationExists ──▶ re-diff

… target

Runs a validated desired schema (CREATE TABLE plus indexes, table always
first) against an AbsentTarget proof: every statement is qualified into
the proof's schema, re-parsed, and admitted by shape and target before
the first step executes (ST-7). Indexes build plainly, not CONCURRENTLY —
the table is born this run with no traffic, and a plain build cannot
leave an INVALID index. SQLSTATE 42P07 maps to the typed
ErrCreateCollision so the caller re-diffs rather than assumes.

Adds the off-ladder CheckCreatePrivileges/CreationRole proof: a
greenfield table has no owner to be a member of, so the create path
proves CONNECT + schema USAGE + CREATE, deliberately not the ownership
tier ladder. Dormant until the declarative front door routes to it.
@Kiran01bm
Kiran01bm marked this pull request as ready for review August 28, 2026 07:55
@chatgpt-codex-connector

Copy link
Copy Markdown

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

Qualify rewrites only the target name, so INHERITS/LIKE/OF bind their
secondary relation or type via search_path to existing objects the
absence proof does not cover - refuse them at admission. Also refuse
in-set duplicate names before anything runs (decidable at admission,
previously a mid-run failure with a committed prefix), and map SQLSTATE
42710 - a standalone type holding the table's name - to the typed
collision alongside 42P07.

@aparajon aparajon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Adversarial correctness review, requested by @aparajon and performed by their agent. Reviewed at head e95a755, in a worktree, with a local build, the full executor / preflight / statement suites, four probes against a real server, and fifteen mutants aimed at the admission guards.

Verdict: the shape of this is right and I'd land it. Ordering the CREATE TABLE first, refusing the clauses that bind to a second relation, running each step as its own bounded transaction, and returning the committed prefix rather than pretending the run was atomic — those are the four decisions that matter and all four are made correctly. The distinction between ErrDuplicateCreateName (the author wrote a conflict; decidable before anything runs) and ErrCreateCollision (the world changed; re-diff) is the right axis to split on, and it is the one an orchestrator actually needs.

Three findings. The first is the one I'd fix before this path goes anywhere near a front door: the executor picks the target's schema from a proof, then executes the statement in a session that has never heard of that schema.

# Finding Severity
1 The create step runs under the session's search_path, not the proof's schema — a type in the target schema is invisible, and a same-named type in public wins silently medium
2 The CREATE TABLE claims pg_class names the duplicate-name map never sees, so a decidable conflict still fails mid-run with a committed prefix medium
3 CreationRole is minted and never consumed — ExecuteCreate takes the absence proof and nothing else, while three docs now list it as a proof dangerous APIs accept medium
4 The unnamed-index exemption in the claim map is reachable and unpinned low (test)
5 Two admission guards can't be reached through the public API, but the docs advertise one of them as a create-path refusal nit

1. Qualify moves the target; the session still resolves everything else

admitCreateStep qualifies the target relation into at.Schema() and re-checks it, which is exactly right for the table. But statement.Qualify rewrites one RangeVar, and executeNativeAttempt sets exactly two things on the transaction — lock_timeout and statement_timeout. Every other name inside the statement is resolved by whatever search_path the pool's session happens to carry.

The header comment already contains the argument. It's why INHERITS, LIKE and 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." That reasoning is correct and it is not specific to clauses that take a lock. A column type, a CHECK function, a DEFAULT nextval(...), an index opclass or expression — all of them are secondary names, none are refused, and none are qualified.

Two probes against a real server, with an AbsentTarget minted for schema S:

CREATE TYPE S.my_enum AS ENUM ('a','b');
ExecuteCreate(..., "CREATE TABLE t (id int, st my_enum)")
  → ERROR: type "my_enum" does not exist (SQLSTATE 42704)

The type is in the proof's own schema and the create still can't see it. That is a valid desired file for that schema that this path cannot execute — and limitations.md explicitly says a column may use an unmanaged type ("A column may use an unmanaged type (an enum, a domain) — the type text round-trips"), so this isn't an exotic input, it's a documented supported one.

The second probe is the one that worries me, because it succeeds:

CREATE TYPE S.my_enum2       AS ENUM ('target');
CREATE TYPE public.my_enum2  AS ENUM ('elsewhere');
ExecuteCreate(..., "CREATE TABLE t (id int, st my_enum2)")
  → err = <nil>
  → column st bound to the type in schema "public"; proof schema was S

The run reports success, and the table it created is not the table the desired file describes. Nothing downstream catches it either: introspection reads under SET LOCAL search_path = <schema>, public (pkg/schemadiff/introspect.go:53), decompiles the type as a bare my_enum2, and a text diff compares equal.

That asymmetry is also the cheapest remedy: the read path already pins search_path for exactly this reason, and the write path — the one that decides where a relation is born — doesn't. A create-specific attempt that issues SET LOCAL search_path = <at.Schema()> alongside the budgets closes both probes; whether it should be <schema> alone or <schema>, public is a policy call worth making explicitly rather than inheriting from the pool. The alternative — refusing any unqualified secondary name at admission — is stricter than the engine wants, given unmanaged types are supported on purpose.

Everything else in this PR treats "the session's search_path is not the proof's schema" as the core hazard. This is the one place the code still assumes they agree.

2. The claim map doesn't see the names the CREATE TABLE mints

admitCreateSteps says it well: "Every step claims a name in the same pg_class namespace, 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 map records the table name and each explicit index name. It does not record the names PostgreSQL mints for the table's own constraint indexes, which live in that same namespace.

CREATE TABLE t (id int PRIMARY KEY);
CREATE INDEX t_pkey ON t (id);

Admission sees t and t_pkey, finds no duplicate, admits. Then:

err  = sequence step 2 of 2 (brief) failed; steps before it committed and their state remains:
       a name the create path needs is already taken: ERROR: relation "t_pkey" already exists (SQLSTATE 42P07)
code = create-collision
steps committed = 1, table exists = true
rerun CheckTableAbsent → ErrRelationExists

So the conflict lands as ErrCreateCollision — "a concurrent create won the race", per the sentinel's own doc — when in fact nothing raced; the file conflicted with itself, decidably, before the first step. And it lands after a commit, in the exact state ErrDuplicateCreateName exists to prevent: a half-created target that now needs an operator re-diff.

The implicit names are deterministic enough to add: the constraint's name when the file gives one, else <table>_pkey for the primary key and <table>_<cols>_key for a UNIQUE. Seeding those into claimed alongside the table name turns this back into an admission-time refusal. If you'd rather not model PostgreSQL's naming, the honest alternative is to narrow the comment — say the map covers explicitly-named relations, and that a collision with an implicit constraint index surfaces at run time as ErrCreateCollision.

3. Nothing consumes the proof this PR adds

CheckCreatePrivileges mints a CreationRole, and grep finds no parameter of that type anywhere in the tree. Meanwhile this PR adds it to all three proof-type lists — SAFETY.md ("dangerous APIs accept only proof types"), .agents/checks/review.md ("Dangerous APIs accept proof types … with package-private constructors"), and docs/tcb-model.md — and pkg/preflight/docs_test.go now pins that it appears in all three.

The dangerous API on this path is ExecuteCreate, and its signature is (ctx, pool, at preflight.AbsentTarget, ds, b, retry). ExecuteNative and RunSequence both take a PreflightedTable. So the create path is the one dangerous entry point in the package that takes a proof for one precondition and simply trusts the caller on the other: a caller can run ExecuteCreate having never checked privileges, and the failure is the server's 42501 rather than the *PrivilegeError carrying GRANT CREATE ON SCHEMA … TO ….

Two ways out, and I don't think it matters much which: take a CreationRole and re-verify cr.Schema() == at.Schema() the way ST-7 re-verifies the other two proofs, or state plainly in ExecuteCreate's doc that the privilege check is the caller's ordering obligation and that the proof is deliberately not threaded. What shouldn't stand is a proof type promoted into the TCB documentation with no consumer — the docs test now guarantees the documentation stays in sync while the type stays inert.

4. The unnamed-index exemption is load-bearing and untested

Mutating if name != "" to if true { leaves the suite green. The exemption is genuinely reachable — a desired file may leave indexes unnamed, and today that works:

CREATE TABLE t (id int, nm text);
CREATE INDEX ON t (id);
CREATE INDEX ON t (nm);
  → 3 steps, t_id_idx and t_nm_idx both built

With the exemption gone, both index steps claim "", and that perfectly legal file is refused with ErrDuplicateCreateName — a false refusal of valid input, invisible to the suite. One subtest with two unnamed indexes closes it, and it pairs naturally with whatever you do for finding 2, since both are about which names the map is supposed to see.

5. Nit: two guards the public API can't reach

op.Concurrent and len(ops) != 1 both survive removal, and I don't think either is a defect — they're re-verification at the point of use, which is the house style. But ParseDesired refuses CONCURRENTLY with ErrConcurrentIndex before a DesiredSchema can exist, so the create path's own concurrent-build refusal is unreachable through the only door that mints its input. capabilities.md, limitations.md and the PR summary all list CONCURRENTLY among the create path's admission refusals; an integrator who reads that and writes errors.Is(err, executor.ErrUnsupportedCreateStep) will get statement.ErrConcurrentIndex instead. Naming the actual owner of that refusal in the two docs is the whole fix.


Action items

  1. (Finding 1) Pin search_path to the proof's schema for the create steps, or refuse unqualified secondary names at admission. Make the <schema> vs <schema>, public choice explicitly.
  2. (Finding 2) Seed the implicit constraint-index names into claimed, or narrow the comment to explicitly-named relations and say the rest surfaces as ErrCreateCollision.
  3. (Finding 3) Either thread CreationRole into ExecuteCreate, or document in ExecuteCreate that the privilege check is a caller ordering obligation.
  4. (Finding 4) Add a two-unnamed-index subtest.
  5. (Nit) Attribute the CONCURRENTLY refusal to ParseDesired in capabilities.md and limitations.md.
Verified — tried to break, couldn't

Long identifiers are handled, and I expected them not to be. Postgres truncates at 63 bytes, so two index names differing only past that byte would collide at run time after a prefix committed — the finding-2 failure with a different cause. It doesn't happen: the parser truncates before ParseOps reports the name. A 69-byte and a 70-byte name sharing a 63-byte prefix both arrive as the same 63-byte string and the claim map refuses the set, which is precisely what the server would have done.

PARTITION OF … DEFAULT sets Partbound, so it refuses as ErrPartitionOfUnsupported rather than falling through to the INHERITS branch with a misleading code. Inherits and PartitionOf are correctly disjoint — the grammar puts the partitioned parent in InhRelations too, and the Partbound == nil guard keeps them apart.

The two ST-7 target checks are mutually redundant but the pair is pinned. Removing the per-step check in admitCreateStep survives, and removing ds.Table() != at.Table() in executeCreate survives — each alone is covered by the other. Removing both fails TestExecuteCreateRefusesProofTargetMismatch, so the redundancy is deliberate depth rather than an untested guard.

The collision mapping is right about implicit names. When the CREATE TABLE itself trips 42P07 on a constraint index, the server's error names the occupant (relation "t_pkey" already exists), so wrapping with the classification rather than an identifier — as the comment says — keeps the message accurate for a collision the executor never explicitly claimed.

A collision is never retried. executeWithLockRetryObserved retries only a BudgetError with CauseLock, so 42P07 and 42710 return on the first attempt. A create is never re-run against a name that just refused it.

The committed-prefix contract holds end to end, including on my own probes: the failing step's SequenceStepError carries Step/Total, rep.Steps covers exactly the prefix, the relations from that prefix are really there, and a rerun's CheckTableAbsent refuses with ErrRelationExists. The rerun story is a real story, not an aspiration.

Step counts agree. tracker.Start(len(ds.Statements())) is called before admission and admitCreateSteps returns exactly one step per desired statement (one CREATE TABLE plus the rest), so the tracker's total and SequenceStepError.Total can't drift.

Reordering is necessary, not cosmetic. ParseDesired admits an index-before-table file — it only checks that every index targets the single CREATE TABLE — so the create-first ordering is what makes those files work, and the mutant that reverses it dies.

Fifteen mutants, nine killed. The four survivors are findings 4 and 5 plus the mutually-redundant ST-7 pair above; every admission refusal that is reachable through ParseDesiredIF NOT EXISTS on both shapes, INHERITS, LIKE, OF, PARTITION OF, the duplicate-name check, the 42710 arm of the collision mapping, and the ordering — dies when removed.

No test deletions. The single removed line is the proof-type list being rewritten to add CreationRole.

Terminology and privilege wording. No "migration" anywhere in the diff. TierCreateTable is added past the end of the ladder, and CheckPrivileges's existing range guard rejects it as out of range (req.Tier > TierCopyAndSwap) — which is the one placement where being the highest value is safe rather than dangerous, so the "checked by CheckCreatePrivileges, never by CheckPrivileges" claim is enforced rather than merely documented. The TierConnect attribution on the schema-USAGE refusal is preserved by the extracted helper, so the two callers keep reporting the same tier they did before.

Locally at head: go build ./... and go test ./pkg/executor/... ./pkg/preflight/... ./pkg/statement/... green; worktree restored clean after every mutant. CI green on PostgreSQL 14 through 18.

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

@aparajon aparajon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Second pass on the same head (e95a755), requested by @aparajon and performed by their agent — this one steps back from correctness and looks at the PR through two lenses: how easy this is to adopt as an OSS library, and what an orchestrator adapter has to do with it. Nothing here blocks; the correctness review above carries the findings that do.

The headline: the code lands four new typed sentinels, four outcome codes, and a partial-failure contract — and docs/schemabot-integration.md, the document an adapter author reads first, still describes the create path as if none of them exist.


Lens 1 — adopting this as a library

What's good, and worth keeping. The "not user-reachable yet" line is stated in README.md, docs/limitations.md and docs/capabilities.md, all three in the same words. A reader who finds ExecuteCreate in the API and wonders why the CLI won't do it gets an answer in the first place they look. That discipline is rare and it's the reason this repo reads as trustworthy.

The ErrDuplicateCreateName / ErrCreateCollision split is the other thing I'd call out as an adoption win. They are the same SQLSTATE from the server's point of view, and they are completely different problems for the user: one is "your file contradicts itself, fix it and rerun", the other is "the database changed under you, re-diff". Most tools collapse those. Splitting them at admission means the caller never has to guess.

The gap: there is no create-path recipe. To use this today an integrator must assemble the call order — ParseDesiredCheckCreatePrivilegesCheckTableAbsentExecuteCreate — from four separate documents plus the package API, and nothing states that the two preflights must run in the same session as the executor. docs/schemabot-integration.md says that rule for AbsentTarget (minted and consumed in the apply session, never serialized) but it predates both CheckCreatePrivileges and the executor, so CreationRole has the same session-scoping property with no equivalent sentence. Ten lines of Go in that section, showing the order and the session, is the whole fix, and it would double as the answer to correctness finding 3.

Outcome codes still have no canonical list. Four new ones land here (create-collision, duplicate-create-name, partition-of-unsupported, unsupported-create-step) and none appear in docs/. docs/optimistic-attempt.md cites a few by example and docs/low-level-design.md names the mechanism, so an integrator writing a switch on OutcomeCode has to read code.go to know what the arms are. This is the docs-pinning ask I've raised on previous PRs and I won't belabour it — but it's now four codes larger, and pkg/preflight/docs_test.go shows the repo already knows how to pin a list like this to the docs mechanically.

Terminology is clean. No "migration" anywhere in the diff.


Lens 2 — what an orchestrator adapter has to do with this

The routing table is now stale, and it's the load-bearing doc. docs/schemabot-integration.md §"Routing the create path's refusals" is where an adapter author decides which failures retry, which fail the apply, and which go back to the PR author. Its last row reads "Duplicate-name error from the CREATE itself" — that error now has a name and a code, and three of its siblings have no row at all. The four rows I'd add:

Refusal What it means Orchestrator action
executor.ErrCreateCollision / create-collision A name the create needed was taken — a concurrent create won the race, or an occupant sits at an index name (index names are never absence-checked) Re-diff the live catalog; never assume the occupant's shape, never blindly retry
executor.ErrDuplicateCreateName / duplicate-create-name The desired file claims one name twice; refused before anything ran, so the target is untouched Author error — surface it on the PR, don't retry, don't fail the environment
executor.ErrPartitionOfUnsupported / partition-of-unsupported The desired file uses a shape outside the create path Deterministic refusal; the same file will always refuse
executor.ErrUnsupportedCreateStep / unsupported-create-step Same, for INHERITS / LIKE / OF / IF NOT EXISTS Deterministic refusal
*preflight.PrivilegeError (tier TierCreateTable) The engine role lacks USAGE or CREATE on the schema Operator action — render the carried GRANT verbatim; retrying cannot succeed

The step number is a safety signal, not a detail. SequenceStepError distinguishes step 1 from step n in its own message, and that distinction is the whole difference between "nothing happened to the target" and "the target now holds relations that need reconciling". An adapter that renders both as "apply failed" loses it. Step, Total and the []StepReport prefix (JSON-tagged already, so it serializes cleanly onto a change record) are exactly the disclosure an orchestrator needs to say what already exists on the target.

This lines up well with the merge-gate rule that a started apply stays authoritative until an operator reconciles: a create that fails at step 3 leaves real relations, the rerun's CheckTableAbsent refuses with ErrRelationExists, and the correct behaviour is to keep blocking rather than let a later commit that drops the table from the desired file make the gate pass by cleanup alone. The engine's contract supports that; it's worth one sentence in the integration doc saying so, because the naive adapter reading is "failed apply → clear state and move on".

Render the code, log the error. SequenceStepError.Error() interpolates the raw server error — ERROR: relation "t_pkey" already exists (SQLSTATE 42P07) in my probe. That particular text is harmless, but a step can fail on a dial error or a permission failure whose text carries host and role detail, and piping err.Error() into a PR comment is how that reaches a public timeline. The pieces to render are already separated: OutcomeCode(err), Step/Total, and the step's own SQL (which is the user's own DDL). One line in the integration doc pointing at that split would stop the obvious mistake before an adapter makes it.

CheckCreatePrivileges is directly reusable for onboarding. It returns the exact provisioning statement, grantee set to the engine role, with no Hint to reword — which is the shape an onboarding preflight wants to print verbatim. Worth pointing at from the engine-role doc's provisioning section as well as the off-ladder section this PR adds, since the two audiences (an operator provisioning a role, an adapter routing a refusal) arrive from different directions.

Progress is step-indexed, not step-named. The tracker gets OperationAdmitting then OperationBrief with a step index, so a progress comment can say "step 2 of 3" but not "building index t_name_idx". Fine for now — noting it because the create path is the first one where the step list is known in full before execution starts, so naming steps is cheaper here than anywhere else.


Suggested follow-ups

  1. Add the four routing rows (plus the *PrivilegeError row) to docs/schemabot-integration.md, replacing the untyped duplicate-name row.
  2. Add a short create-path recipe to the same document: the call order, and that both proofs are session-scoped and consumed in the apply session.
  3. State the committed-prefix reconciliation expectation — a failed create leaves relations, and the gate stays closed until an operator reconciles.
  4. Note that adapters should render OutcomeCode + step position + the step's SQL, and log the raw error rather than surfacing it.
  5. (recurring) Give the outcome codes a canonical documented list, pinned the way pkg/preflight/docs_test.go pins the proof types.

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

@aparajon aparajon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Approving on @aparajon's behalf after the adversarial correctness review above (no blocking findings). This stamp was left by Claude Code (claude-opus-5).

@Kiran01bm

Copy link
Copy Markdown
Collaborator Author

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

Response to both adversarial reviews at head e95a755 (correctness F1–F5 and the non-blocking design review). All five correctness findings and all five design suggestions are addressed in the follow-up commit.

Correctness findings

# Finding Resolution
F1 Create steps ran under the session search_path, not the proof's schema Fixed — each create step now pins SET LOCAL search_path = <schema>, public inside its bounded transaction, the same policy the introspection read path sets, so unqualified type references resolve exactly as the diff resolved them. Integration test creates the type in the target schema plus a decoy in public and asserts udt_schema is the proof's schema.
F2 The admission claim map missed implicit constraint-index names (t_pkey, <table>_<cols>_key) Fixed — new statement.ImplicitIndexNames predicts the server's first-choice names for PRIMARY KEY / UNIQUE / EXCLUDE (named constraints verbatim; unnamed ones via a faithful port of makeObjectName, including the 63-byte truncation), and CREATE TABLE admission seeds them into the claim map. Deliberately fail-closed: identical first choices are refused even where the server would suffix around them. Pinned by a two-oracle integration test comparing predictions against the names the real server mints, plus collision refusal tests.
F3 The CreationRole proof was minted but consumed by nothing Fixed — ExecuteCreate now takes the CreationRole and re-verifies it under ST-7: a zero proof and a schema mismatch with the absence proof are both ErrInvariantViolation, integration-tested.
F4 The name != "" unnamed-index exemption was unpinned by tests Fixed — new integration test runs a desired set with two unnamed CREATE INDEX steps and asserts both server-named indexes exist.
F5 Docs attributed the CONCURRENTLY refusal to create-path admission, but it is owned upstream by statement.ParseDesired Fixed — capabilities.md and limitations.md now attribute REFERENCES and CONCURRENTLY to desired-file parse, with the executor's check named as defense in depth.

Design review (non-blocking)

# Suggestion Resolution
1 Routing-table rows for the create path's typed refusals Added to schemabot-integration.md: a second table maps ErrDuplicateCreateName, ErrPartitionOfUnsupported, ErrUnsupportedCreateStep, ErrCreateCollision, and *preflight.PrivilegeError (TierCreateTable) to distinct orchestrator actions.
2 Document the create-path call recipe Added: ParseDesiredCheckCreatePrivilegesCheckTableAbsentExecuteCreate, with both proofs minted and consumed inside the apply session, never serialized.
3 Committed-prefix reconciliation expectation Added: a failed create leaves its committed steps; the gate stays closed until the front door re-diffs the live catalog — the orchestrator never assumes the failed run left nothing behind.
4 Note what adapters render vs. log Added to the new outcome-codes section in execution-model.md: adapters render the outcome code, step position, and step SQL; the raw error is logged, not branched on.
5 Pin the canonical outcome-code list mechanically Added executor.Codes() enumerating the closed vocabulary, an outcome-codes table in execution-model.md, and a docs test that fails if any code is missing from the page (plus a uniqueness test).

Pin create-step search_path to the proof's schema so unqualified type
references resolve as the diff resolved them; seed first-choice implicit
constraint-index names into the admission claim map (two-oracle-tested
against the server's makeObjectName); consume the CreationRole proof in
ExecuteCreate with ST-7 re-verification; pin the unnamed-index exemption;
re-attribute the CONCURRENTLY refusal to desired-file parse; document the
outcome-code vocabulary (pinned by a docs test via executor.Codes) and
the create path's refusal routing for orchestrators.
@Kiran01bm
Kiran01bm merged commit 6f3b20e into main Aug 28, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants