executor: run planner-produced safer sequences natively (Phase 3.2) - #28
Conversation
Adds the sequence executor for the remaining native idioms: NOT VALID + VALIDATE CONSTRAINT, ADD PRIMARY KEY USING INDEX over a concurrent build, the four-step SET NOT NULL, fast-default, and metadata-only changes. Every step is re-admitted by the real grammar and bound to the preflighted table before anything executes; each step class carries its own budget, and a failed step surfaces as a typed *SequenceStepError naming the committed prefix.
Refusals decidable at admission must never fire mid-run after a committed prefix: the delegated build executor's static checks and pool guard now run before the first step, and over-ceiling budget values are refused as a unit. An external cancel of a long validation is corroborated by elapsed time so it never reads as budget exhaustion. Registry and design docs amended to match.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
morgo
left a comment
morgo
left a comment
There was a problem hiding this comment.
🤖 Approved by Morgan's AI agent (moderate-risk bar for pg-sprite): sequence executor is fail-closed the whole way — full-sequence admission (re-parse, shape classification, ST-7 target proof, pool guard) before the first step runs, per-class budgets, typed SequenceStepError keeping the underlying BudgetError/PgError reachable, and the external-cancel vs statement-timeout disambiguation on the validate class is a genuinely nice touch. Integration tests prove the planner→executor seam including resume-from-failed-step. Non-blocking heads-up: this calls AttemptNative, which #27 renames to ExecuteNative — whichever merges second needs a small rebase (both also touch optimistic.go's Budget.validate).
…-idioms * origin/main: surface retry attempts and prove lock-retry flag wiring feat(executor): lock_timeout + bounded retry for native DDL # Conflicts: # docs/invariants.md
2 participants
Summary
Adds the Phase 3.2 native sequence executor to
pkg/executor:RunSequenceruns a planner-produced safer sequence under the autocommit-each-step contract. This completes the native idioms whose safety comes from their sequencing —NOT VALID+VALIDATE CONSTRAINT,ADD PRIMARY KEY USING INDEXover a concurrent build, the four-stepSET NOT NULL, fast-defaultADD COLUMN, and metadata-only changes.What
RunSequence(ctx, pool, preflightedTable, steps, SequenceBudget): admits the whole sequence before executing any step — every step re-parsed by the real grammar, shape-classified, and verified against the preflight proof (ST-7), so a sequence the executor cannot finish is never started.CREATE INDEX CONCURRENTLYdelegates to the existing concurrent build executor, and a loneVALIDATE CONSTRAINTgets a dedicatedValidateBudget(tight lock wait, generous scan bound).ErrUnsupportedSequenceStep,ErrEmptySequence), including every otherCONCURRENTLYform.*SequenceStepErrorcarrying the step position, class, and the committed prefix; the underlying*BudgetError/*InvalidIndexError/*pgconn.PgErrorstays reachable througherrors.Is/As.planner.Classifyconstructs, proving the planner-to-executor seam end to end, including partial-failure state and budget cancellation of a blocked step.Why
The classifier has constructed these safer sequences since Phase 2, but only
diffandmigrate --dry-runcould render them — nothing could execute them. This lands the execution engine; wiring the CLI front door to route classified sequences here is the next Phase 3 step (tracked in the low-level design's remaining-work list).This PR was authored by Amp (Claude Opus 4.6).