diff --git a/docs/invariants.md b/docs/invariants.md index 5dc1669..17a82c5 100644 --- a/docs/invariants.md +++ b/docs/invariants.md @@ -418,6 +418,17 @@ Each refusal is a preflight **error with a stated reason** — never a warning, fields; the classification registry (`pkg/plan/refusal.go`, `pkg/migrate/refusal_registry.go`) is checked for completeness against the production closed sets by `TestRefusalRegistryIsComplete`. *Source:* [refusal classes](refusal-classes.md). +- **RF-8** — An in-process refused verdict retains its full typed refusal proof, and + `Verdict.Refusal` hands that proof out only when it validates under RF-7 and still matches + the verdict's exported refusal fields. A verdict decoded from JSON reconstructs class, + reason, owner, and cause, but cannot recover the unexported refusal site: the site-keyed + eligibility row fails closed for a decoded verdict, while the cause-keyed row is decidable + from the wire fields. Accepted-blocking eligibility is therefore consumed only from the + proof of the verdict the same front-door invocation produced, never from a decoded verdict. + *Enforced:* `Verdict.WithRefusal`, `Verdict.Refusal`, + `TestRefusalRejectsUnvalidatedProof`, `TestRefusalRejectsFieldsDivergingFromProof`, and + `TestGateVerdictAcceptedBlockingEligibility`, which pins that the site-keyed row is eligible + in process and ineligible after a JSON round trip. *Source:* [lock-budgeted passthrough](lock-budgeted-passthrough.md). ## Orchestration / control-plane (OC) diff --git a/docs/lock-budgeted-passthrough.md b/docs/lock-budgeted-passthrough.md index 5763cb6..9a694e0 100644 --- a/docs/lock-budgeted-passthrough.md +++ b/docs/lock-budgeted-passthrough.md @@ -102,6 +102,17 @@ it is acquired. | 6 | Eligibility is selected from a closed registry keyed by typed class, reason, and cause or refusal site. No renderer text or SQL substring participates in the decision. | | 7 | A dry-run reports the original refusal and a per-statement `blocking_passthrough_eligible` boolean, but executes nothing even when the flag is present. | +The full typed refusal proof is available while its verdict remains in process, and +`Refusal()` returns it only when it validates as a classified refusal and still matches the +verdict's exported fields. JSON carries class, reason, owner, and cause, but not the internal +refusal site. Calling `Refusal()` on a decoded verdict therefore reconstructs the JSON fields +but cannot restore the site: the site-keyed row (single-relation `index-statement`) fails +closed after decoding, while the cause-keyed row (`unsupported-partitioned-parent` with +`parent-blocking-index-build`) is decidable from the wire fields. Decoding is not the +fail-closed boundary; the front door is. Eligibility is consumed only from the proof of the +verdict the same `migrate` or `diff` invocation produced, never from a verdict read back +from JSON. + “Prints before execution” is an ordering requirement for human output and an information requirement for JSON. Human mode prints the refusal analysis, then a separate acceptance line, then starts the session. JSON remains one final machine-readable object; its executed verdict @@ -373,7 +384,7 @@ Sequence implementation as follows: cause or refusal site. The `index-statement` site distinguishes the single-relation forms from `DROP INDEX a, b` and `REINDEX SCHEMA`, `DATABASE`, and `SYSTEM`, so the registry can admit the former and refuse the latter without a database. Its completeness tests make new - values ineligible by default and prove that render text is never consulted. + values ineligible by default and prove that render text is never consulted. *(done)* 2. Add the executor path through engine-owned bounded sessions, requiring explicit non-zero `statement_timeout` and non-zero `lock_timeout`. At this step add lock-budget invariants to [the invariant registry](invariants.md): every passthrough statement runs in an diff --git a/internal/cli/color_test.go b/internal/cli/color_test.go index 601abf2..5259e4e 100644 --- a/internal/cli/color_test.go +++ b/internal/cli/color_test.go @@ -182,6 +182,9 @@ func fullVerdict(t *testing.T) verdict.Verdict { } rv := reflect.ValueOf(v) for i := range rv.NumField() { + if rv.Type().Field(i).PkgPath != "" { + continue + } require.False(t, rv.Field(i).IsZero(), "Verdict field %s is zero in the all-fields fixture; set it so the renderer parity lock covers it", rv.Type().Field(i).Name) diff --git a/pkg/migrate/refusal_registry.go b/pkg/migrate/refusal_registry.go index 56046d2..53ea9fe 100644 --- a/pkg/migrate/refusal_registry.go +++ b/pkg/migrate/refusal_registry.go @@ -102,7 +102,7 @@ func siteRefusals() []siteRefusal { // (ALTER TABLE, CREATE INDEX). concurrent distinguishes the already-safe // maintenance forms — which pg-sprite need not wrap — from the plain forms // it refuses in favor of their concurrent idiom. -func gateRefusal(kind statement.Kind, concurrent bool) (verdict.Refusal, bool) { +func gateRefusal(kind statement.Kind, concurrent bool, target statement.IndexTarget) (verdict.Refusal, bool) { switch kind { case statement.KindAlterTable, statement.KindCreateIndex: return verdict.Refusal{}, false @@ -110,7 +110,11 @@ func gateRefusal(kind statement.Kind, concurrent bool) (verdict.Refusal, bool) { if concurrent { return verdict.NoOnlineSafetyProblem(verdict.ReasonIndexStatement, verdict.OwnerDirectOperator), true } - return verdict.ByDesign(verdict.ReasonIndexStatement), true + site := verdict.RefusalSiteIndexOther + if target == statement.IndexTargetSingleRelation { + site = verdict.RefusalSiteIndexSingleRelation + } + return verdict.ByDesign(verdict.ReasonIndexStatement).WithSite(site), true case statement.KindCreateTable: return verdict.NoOnlineSafetyProblem(verdict.ReasonUnsupportedStatement, verdict.OwnerDeclarativeFrontDoor), true case statement.KindDataChange: @@ -201,6 +205,48 @@ func partitionRefusal(cause preflight.PartitionRefusalCause) (verdict.Refusal, b return plan.PartitionRefusal(cause) } +// AcceptedBlockingEligible reports whether a typed refusal is in the closed +// accepted-blocking registry. Unknown combinations fail closed. +func AcceptedBlockingEligible(r verdict.Refusal) bool { + eligible, decided := acceptedBlockingDecision(r) + return decided && eligible +} + +// acceptedBlockingDecision is total over the registered reason and +// cause/site vocabulary. The bool pair is eligibility and whether the key +// has an explicit decision; completeness tests reject undecided additions. +func acceptedBlockingDecision(r verdict.Refusal) (bool, bool) { + switch r.Reason() { + case verdict.ReasonIndexStatement: + switch r.Site() { + case verdict.RefusalSiteIndexSingleRelation: + return r.Class() == verdict.ClassByDesign, true + case verdict.RefusalSiteIndexOther, "": + return false, true + default: + return false, false + } + case verdict.ReasonUnsupportedPartitionedParent: + switch r.Cause() { + case verdict.CauseParentBlockingIndexBuild: + return r.Class() == verdict.ClassCapabilityBoundary, true + case verdict.CauseParentConcurrentIndexBuild, verdict.CauseParentIndexAdoption, + verdict.CauseParentNotValidForeignKey: + return false, true + default: + return false, false + } + case verdict.ReasonUnsupportedStatement, verdict.ReasonTableTooLarge, + verdict.ReasonInsufficientPrivileges, verdict.ReasonBudgetExceeded, + verdict.ReasonRewriteRequired, verdict.ReasonBackendUnavailable, + verdict.ReasonDestructiveChange, verdict.ReasonPlanFingerprintMismatch, + verdict.ReasonCreateCollision: + return false, true + default: + return false, false + } +} + // isInSentinelSet reports whether err matches any sentinel in set. func isInSentinelSet(err error, set []error) bool { for _, sentinel := range set { diff --git a/pkg/migrate/refusal_registry_test.go b/pkg/migrate/refusal_registry_test.go index 24c4b03..9b435e7 100644 --- a/pkg/migrate/refusal_registry_test.go +++ b/pkg/migrate/refusal_registry_test.go @@ -1,6 +1,7 @@ package migrate import ( + "encoding/json" "errors" "fmt" "testing" @@ -35,7 +36,7 @@ func deriveRefusalKeys() (keys []classifiedKey, admitted []string) { keys = append(keys, classifiedKey{"create-shape:" + string(c), r, ok}) } for _, c := range preflight.PartitionRefusalCauses() { - r, ok := plan.PartitionRefusal(c) + r, ok := partitionRefusal(c) keys = append(keys, classifiedKey{"partition:" + string(c), r, ok}) } for _, err := range admissionSentinels() { @@ -48,13 +49,22 @@ func deriveRefusalKeys() (keys []classifiedKey, admitted []string) { } for _, k := range statement.Kinds() { for _, concurrent := range []bool{false, true} { - r, ok := gateRefusal(k, concurrent) - key := fmt.Sprintf("gate:%s/concurrent=%t", k, concurrent) - if !ok { - admitted = append(admitted, key) - continue + targets := []statement.IndexTarget{statement.IndexTargetNone} + if !concurrent && (k == statement.KindDropIndex || k == statement.KindReindex) { + targets = statement.IndexTargets() + } + for _, target := range targets { + r, ok := gateRefusal(k, concurrent, target) + key := fmt.Sprintf("gate:%s/concurrent=%t", k, concurrent) + if len(targets) > 1 { + key = fmt.Sprintf("%s/target=%s", key, target) + } + if !ok { + admitted = append(admitted, key) + continue + } + keys = append(keys, classifiedKey{key, r, ok}) } - keys = append(keys, classifiedKey{key, r, ok}) } } for _, s := range siteRefusals() { @@ -105,6 +115,9 @@ func TestRefusalRegistryIsComplete(t *testing.T) { // and owner rule. _, err := verdict.NewRefusal(k.refusal.Class(), k.refusal.Reason(), k.refusal.Owner()) require.NoError(t, err) + _, decided := acceptedBlockingDecision(k.refusal) + require.True(t, decided, "eligibility registry has no explicit decision for class=%q reason=%q cause=%q site=%q", + k.refusal.Class(), k.refusal.Reason(), k.refusal.Cause(), k.refusal.Site()) classified[k.refusal.Reason()] = true }) } @@ -126,15 +139,15 @@ func TestRefusalRegistryCorrespondence(t *testing.T) { assert.Equal(t, verdict.ClassByDesign, class(plan.CreateShapeRefusal(executor.CreateShapeIfNotExists))) assert.Equal(t, verdict.ClassCapabilityBoundary, class(plan.CreateShapeRefusal(executor.CreateShapePartitionOf))) assert.Equal(t, verdict.ClassInvariantViolation, class(plan.CreateShapeRefusal(executor.CreateShapeMultipleOperations))) - assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindDataChange, false))) + assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindDataChange, false, statement.IndexTargetNone))) // unsupported-partitioned-parent spans three. assert.Equal(t, verdict.ClassCapabilityBoundary, class(plan.PartitionRefusal(preflight.PartitionCauseConcurrentIndexBuild))) assert.Equal(t, verdict.ClassByDesign, class(plan.PartitionRefusal(preflight.PartitionCauseIndexAdoption))) assert.Equal(t, verdict.ClassEnvironmental, class(plan.PartitionRefusal(preflight.PartitionCauseNotValidForeignKey))) // index-statement: the plain form is refused by design, the concurrent // form is the operator's to run. - assert.Equal(t, verdict.ClassByDesign, class(gateRefusal(statement.KindDropIndex, false))) - assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindReindex, true))) + assert.Equal(t, verdict.ClassByDesign, class(gateRefusal(statement.KindDropIndex, false, statement.IndexTargetSingleRelation))) + assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindReindex, true, statement.IndexTargetSingleRelation))) // Owners: each no-online-safety-problem kind names who runs it. for kind, owner := range map[statement.Kind]verdict.Owner{ statement.KindDataChange: verdict.OwnerDataChangeRunner, @@ -143,11 +156,11 @@ func TestRefusalRegistryCorrespondence(t *testing.T) { statement.KindCreateTable: verdict.OwnerDeclarativeFrontDoor, statement.KindDropIndex: verdict.OwnerDirectOperator, } { - r, ok := gateRefusal(kind, kind == statement.KindDropIndex) + r, ok := gateRefusal(kind, kind == statement.KindDropIndex, statement.IndexTargetSingleRelation) require.True(t, ok, kind) assert.Equal(t, owner, r.Owner(), kind) } - r, ok := gateRefusal(statement.KindOther, false) + r, ok := gateRefusal(statement.KindOther, false, statement.IndexTargetNone) require.True(t, ok) assert.Equal(t, verdict.ClassCapabilityBoundary, r.Class(), "unnamed grammar is a boundary, not someone else's work") assert.Empty(t, r.Owner()) @@ -168,6 +181,157 @@ func TestRefusalRegistryCorrespondence(t *testing.T) { assert.Equal(t, fromCause, fromSentinel) } +// The eligible set is pinned from both sides over the same production walk +// the completeness harness uses: exactly the single-relation DROP INDEX and +// REINDEX gate keys and the blocking parent index build are eligible, and +// every other key the closed sets name — the other partition causes, the +// concurrent and multi-relation index forms, every create-shape cause, +// admission sentinel, site refusal, and the route refusal — is ineligible. +// A registry that flipped a sibling cause or dropped a class check would +// change this set. +func TestAcceptedBlockingEligibleRowsArePinned(t *testing.T) { + keys, _ := deriveRefusalKeys() + require.NotEmpty(t, keys) + + var eligible []string + for _, k := range keys { + if AcceptedBlockingEligible(k.refusal) { + eligible = append(eligible, k.key) + } + } + assert.ElementsMatch(t, []string{ + "gate:DROP INDEX/concurrent=false/target=single-relation", + "gate:REINDEX/concurrent=false/target=single-relation", + "partition:" + string(preflight.PartitionCauseBlockingIndexBuild), + }, eligible) + + for _, cause := range preflight.PartitionRefusalCauses() { + r, ok := partitionRefusal(cause) + require.True(t, ok, cause) + assert.Equal(t, cause == preflight.PartitionCauseBlockingIndexBuild, AcceptedBlockingEligible(r), cause) + } + assert.False(t, AcceptedBlockingEligible(rewriteRequiredRefusal())) + assert.False(t, AcceptedBlockingEligible(backendUnavailableRefusal())) +} + +// The registry's default arms are fail-closed, not merely undecided: a +// reason, site, or cause outside the vocabulary it enumerates is ineligible +// and reported as undecided, so the completeness harness rejects it. The +// per-class constructors do not validate their reason, which is how a +// vocabulary addition reaches the registry before it is classified. +func TestAcceptedBlockingUnknownKeysFailClosed(t *testing.T) { + tests := []struct { + name string + refusal verdict.Refusal + }{ + {"unknown reason", verdict.ByDesign("brand-new-reason")}, + {"unknown site under index-statement", verdict.ByDesign(verdict.ReasonIndexStatement).WithSite("brand-new-site")}, + {"unknown cause under partitioned parent", verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent).WithCause("brand-new-cause")}, + {"no cause under partitioned parent", verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent)}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + eligible, decided := acceptedBlockingDecision(tc.refusal) + assert.False(t, eligible) + assert.False(t, decided) + assert.False(t, AcceptedBlockingEligible(tc.refusal)) + }) + } +} + +// Each eligible row is keyed on its class as well as its reason and +// discriminator: the same reason and site, or reason and cause, under a +// different class is a decided ineligible key, not an eligible one. +func TestAcceptedBlockingRowsRequireTheirClass(t *testing.T) { + tests := []struct { + name string + refusal verdict.Refusal + }{ + {"index site outside by-design", + verdict.NoOnlineSafetyProblem(verdict.ReasonIndexStatement, verdict.OwnerDirectOperator). + WithSite(verdict.RefusalSiteIndexSingleRelation)}, + {"parent cause outside capability-boundary", + verdict.ByDesign(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentBlockingIndexBuild)}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + eligible, decided := acceptedBlockingDecision(tc.refusal) + assert.False(t, eligible) + assert.True(t, decided) + }) + } +} + +func TestIndexStatementAcceptedBlockingEligibility(t *testing.T) { + tests := []struct { + name string + sql string + want bool + }{ + {"drop one index", "DROP INDEX app.i", true}, + {"reindex index", "REINDEX INDEX app.i", true}, + {"reindex table", "REINDEX TABLE app.t", true}, + {"drop multiple indexes", "DROP INDEX app.i, app.j", false}, + {"reindex schema", "REINDEX SCHEMA app", false}, + {"reindex database", "REINDEX DATABASE app", false}, + {"reindex system", "REINDEX SYSTEM app", false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + st, err := statement.ParseOne(tc.sql) + require.NoError(t, err) + r, ok := gateRefusal(st.Kind(), st.Concurrent(), st.IndexTarget()) + require.True(t, ok) + assert.Equal(t, tc.want, AcceptedBlockingEligible(r)) + }) + } +} + +func TestGateVerdictAcceptedBlockingEligibility(t *testing.T) { + for _, tc := range []struct { + name string + sql string + want bool + }{ + {"single relation", "DROP INDEX app.i", true}, + {"multiple relations", "DROP INDEX app.i, app.j", false}, + } { + t.Run(tc.name, func(t *testing.T) { + st, err := statement.ParseOne(tc.sql) + require.NoError(t, err) + v, refused := Gate(st) + require.True(t, refused) + r, err := v.Refusal() + require.NoError(t, err) + assert.Equal(t, tc.want, AcceptedBlockingEligible(r)) + + if tc.want { + encoded, err := json.Marshal(v) + require.NoError(t, err) + var decoded verdict.Verdict + require.NoError(t, json.Unmarshal(encoded, &decoded)) + decodedRefusal, err := decoded.Refusal() + require.NoError(t, err) + assert.False(t, AcceptedBlockingEligible(decodedRefusal)) + } + }) + } +} + +func TestAcceptedBlockingEligibilityIgnoresRenderedText(t *testing.T) { + r, ok := gateRefusal(statement.KindDropIndex, false, statement.IndexTargetSingleRelation) + require.True(t, ok) + first, err := verdict.Verdict{Detail: "first explanation", SaferIdiom: "first rendering"}.WithRefusal(r).Refusal() + require.NoError(t, err) + second, err := verdict.Verdict{Detail: "completely different", SaferIdiom: "different rendering"}.WithRefusal(r).Refusal() + require.NoError(t, err) + + assert.Equal(t, first, second) + assert.Equal(t, AcceptedBlockingEligible(first), AcceptedBlockingEligible(second)) + assert.True(t, AcceptedBlockingEligible(first)) +} + // Membership and classification are one walk: an error outside the sentinel // set, or a sentinel wrapped in a step error (execution started), is not an // admission refusal. diff --git a/pkg/migrate/verdicts.go b/pkg/migrate/verdicts.go index e837286..79faf84 100644 --- a/pkg/migrate/verdicts.go +++ b/pkg/migrate/verdicts.go @@ -21,7 +21,7 @@ import ( // statements are never executed. Gate needs no database, so a caller can // refuse before dialing; [Run] re-checks it regardless. func Gate(st statement.Statement) (verdict.Verdict, bool) { - r, refused := gateRefusal(st.Kind(), st.Concurrent()) + r, refused := gateRefusal(st.Kind(), st.Concurrent(), st.IndexTarget()) if !refused { return verdict.Verdict{}, false } @@ -170,14 +170,20 @@ func admissionRefusalVerdict(st statement.Statement, err error, r verdict.Refusa // different strategy, while a blind attempt that ran past its budget is // doing rewrite work. A refused forced attempt still records the override. func budgetVerdict(st statement.Statement, budgetErr *executor.BudgetError, forced, online bool) verdict.Verdict { + cause := verdict.CauseNone + switch budgetErr.Cause { + case executor.CauseLock: + cause = verdict.CauseLockBudget + case executor.CauseStatement: + cause = verdict.CauseStatementBudget + } v := verdict.Verdict{ Statement: st.SQL(), Table: qualified(st), Forced: forced, - }.WithRefusal(budgetExceededRefusal()) + }.WithRefusal(budgetExceededRefusal().WithCause(cause)) switch budgetErr.Cause { case executor.CauseLock: - v.Cause = verdict.CauseLockBudget v.Attempts = budgetErr.Attempts if budgetErr.Attempts > 1 { v.Detail = fmt.Sprintf("the lock was not granted within the %s lock budget on any of %d bounded "+ @@ -188,7 +194,6 @@ func budgetVerdict(st statement.Statement, budgetErr *executor.BudgetError, forc v.Detail = fmt.Sprintf("the lock was not granted within the %s lock budget: the table is too "+ "contended right now; nothing was executed", budgetErr.Budget) case executor.CauseStatement: - v.Cause = verdict.CauseStatementBudget if online { v.Detail = fmt.Sprintf("cancelled after the %s budget: the statement already is the safe online "+ "idiom — the work needs more time, not a different strategy; retry with a larger budget for "+ diff --git a/pkg/plan/plan_test.go b/pkg/plan/plan_test.go index b0575f4..9e82dd8 100644 --- a/pkg/plan/plan_test.go +++ b/pkg/plan/plan_test.go @@ -205,6 +205,19 @@ func TestRefuseUnsupportedPartitionedParentClassFollowsCause(t *testing.T) { } } +// The refusal proof carries the preflight cause under the verdict package's +// own typed vocabulary. The two constant sets spell the same wire tokens, and +// this pins that correspondence for every cause preflight registers: a cause +// added on one side without the other is a registry hole, not a silent +// no-cause refusal. +func TestPartitionRefusalCarriesItsCause(t *testing.T) { + for _, cause := range preflight.PartitionRefusalCauses() { + r, ok := plan.PartitionRefusal(cause) + require.True(t, ok, cause) + assert.Equal(t, string(cause), string(r.Cause()), cause) + } +} + func TestRefuseUnsupportedPartitionedParentFailsClosed(t *testing.T) { r := plan.Report{Disposition: router.DispositionExecute, Statements: []plan.Statement{{Disposition: router.DispositionExecute}}} require.Error(t, plan.RefuseUnsupportedPartitionedParent(&r, nil), "positional length mismatch") diff --git a/pkg/plan/refusal.go b/pkg/plan/refusal.go index 38ac4a9..e35fe17 100644 --- a/pkg/plan/refusal.go +++ b/pkg/plan/refusal.go @@ -44,21 +44,30 @@ func CreateShapeRefusal(cause executor.CreateShapeCause) (verdict.Refusal, bool) // PartitionRefusal classifies a partitioned-parent refusal by its cause. The // closed key set is preflight.PartitionRefusalCauses(); ok is false outside // it. The four causes span three classes, which is why the plan carries the -// cause rather than a bare refused flag. +// cause rather than a bare refused flag. The refusal keeps its typed cause, +// so every consumer of the proof — both front doors and the accepted-blocking +// eligibility registry — reads one narrowing instead of re-deriving it from +// the preflight error. func PartitionRefusal(cause preflight.PartitionRefusalCause) (verdict.Refusal, bool) { switch cause { - case preflight.PartitionCauseConcurrentIndexBuild, preflight.PartitionCauseBlockingIndexBuild: - // The partition-aware concurrent index flow is a planned capability; - // refusing the blocking substitute is the policy half of the same gap. - return verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent), true + case preflight.PartitionCauseConcurrentIndexBuild: + // The partition-aware concurrent index flow is a planned capability. + return verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentConcurrentIndexBuild), true + case preflight.PartitionCauseBlockingIndexBuild: + // Refusing the blocking substitute is the policy half of the same gap. + return verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentBlockingIndexBuild), true case preflight.PartitionCauseIndexAdoption: // No supported PostgreSQL version adopts an index as a constraint on // a partitioned parent; waiting for an engine release waits for nothing. - return verdict.ByDesign(verdict.ReasonUnsupportedPartitionedParent), true + return verdict.ByDesign(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentIndexAdoption), true case preflight.PartitionCauseNotValidForeignKey: // The same statement runs on a newer server; the unblocking action is // a server upgrade, not an engine release. - return verdict.Environmental(verdict.ReasonUnsupportedPartitionedParent), true + return verdict.Environmental(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentNotValidForeignKey), true default: return verdict.Refusal{}, false } diff --git a/pkg/statement/statement.go b/pkg/statement/statement.go index e014d47..9bae702 100644 --- a/pkg/statement/statement.go +++ b/pkg/statement/statement.go @@ -89,9 +89,44 @@ type Statement struct { schema string table string concurrent bool + indexTarget IndexTarget buildsIndex bool } +// IndexTarget identifies whether an index-maintenance statement names one +// relation. The gate uses this parse-only fact to keep forms requiring more +// than one lock acknowledgement out of accepted-blocking eligibility. +type IndexTarget int + +const ( + // IndexTargetNone is carried by statements outside index maintenance. + IndexTargetNone IndexTarget = iota + // IndexTargetSingleRelation covers one DROP INDEX and REINDEX INDEX or TABLE. + IndexTargetSingleRelation + // IndexTargetOther covers multi-index DROP and REINDEX scopes that do not + // name one relation: SCHEMA, DATABASE, and SYSTEM. + IndexTargetOther +) + +// IndexTargets returns the closed set of index-maintenance target shapes. +func IndexTargets() []IndexTarget { + return []IndexTarget{IndexTargetSingleRelation, IndexTargetOther} +} + +// String returns the stable name of an index target shape. +func (t IndexTarget) String() string { + switch t { + case IndexTargetNone: + return "none" + case IndexTargetSingleRelation: + return "single-relation" + case IndexTargetOther: + return "other" + default: + return fmt.Sprintf("IndexTarget(%d)", t) + } +} + // SQL returns the original statement text as submitted. func (s Statement) SQL() string { return s.sql } @@ -112,6 +147,9 @@ func (s Statement) Table() string { return s.table } // It is always false for non-index kinds. func (s Statement) Concurrent() bool { return s.concurrent } +// IndexTarget returns the typed target shape of an index-maintenance statement. +func (s Statement) IndexTarget() IndexTarget { return s.indexTarget } + // BuildsIndex reports whether executing the statement creates a new index: // every CREATE INDEX, and the ALTER TABLE shapes that build one as a side // effect — ADD CONSTRAINT UNIQUE / PRIMARY KEY / EXCLUDE without USING @@ -218,12 +256,24 @@ func ParseOne(sql string) (Statement, error) { if node.GetDropStmt().GetRemoveType() == pganalyze.ObjectType_OBJECT_INDEX { st.kind = KindDropIndex st.concurrent = node.GetDropStmt().GetConcurrent() + if len(node.GetDropStmt().GetObjects()) == 1 { + st.indexTarget = IndexTargetSingleRelation + } else { + st.indexTarget = IndexTargetOther + } } else { st.kind = KindCatalogWork } case node.GetReindexStmt() != nil: st.kind = KindReindex st.concurrent = reindexConcurrently(node.GetReindexStmt()) + switch node.GetReindexStmt().GetKind() { + case pganalyze.ReindexObjectType_REINDEX_OBJECT_INDEX, + pganalyze.ReindexObjectType_REINDEX_OBJECT_TABLE: + st.indexTarget = IndexTargetSingleRelation + default: + st.indexTarget = IndexTargetOther + } default: // Parsed statements that are recognized catalog operations are direct // operator work. Truly unknown grammar nodes retain KindOther. diff --git a/pkg/statement/statement_test.go b/pkg/statement/statement_test.go index 1a546b5..68597ac 100644 --- a/pkg/statement/statement_test.go +++ b/pkg/statement/statement_test.go @@ -99,27 +99,27 @@ func TestParseOneKinds(t *testing.T) { { name: "drop index", sql: "DROP INDEX idx_users_email", - want: Statement{kind: KindDropIndex}, + want: Statement{kind: KindDropIndex, indexTarget: IndexTargetSingleRelation}, }, { name: "drop index concurrently", sql: "DROP INDEX CONCURRENTLY idx_users_email", - want: Statement{kind: KindDropIndex, concurrent: true}, + want: Statement{kind: KindDropIndex, concurrent: true, indexTarget: IndexTargetSingleRelation}, }, { name: "reindex table", sql: "REINDEX TABLE users", - want: Statement{kind: KindReindex}, + want: Statement{kind: KindReindex, indexTarget: IndexTargetSingleRelation}, }, { name: "reindex index", sql: "REINDEX INDEX idx_users_email", - want: Statement{kind: KindReindex}, + want: Statement{kind: KindReindex, indexTarget: IndexTargetSingleRelation}, }, { name: "reindex table concurrently", sql: "REINDEX TABLE CONCURRENTLY users", - want: Statement{kind: KindReindex, concurrent: true}, + want: Statement{kind: KindReindex, concurrent: true, indexTarget: IndexTargetSingleRelation}, }, { name: "alter index parses as AlterTableStmt but is not a table target", @@ -354,9 +354,46 @@ func TestKindsIsClosedAndNamed(t *testing.T) { "Kinds() must enumerate exactly the Kind constants statement.go declares") } +// IndexTargets() is the closed set the eligibility registry walks, so it +// must name every declared target shape except IndexTargetNone, which +// marks statements outside index maintenance rather than a shape of one. +// Each named shape has its own String() rather than the numeric fallback. +func TestIndexTargetsIsClosedAndNamed(t *testing.T) { + targets := IndexTargets() + assert.NotContains(t, targets, IndexTargetNone, "the non-shape is not a target the registry decides") + + declared := declaredIotaValues(t, "IndexTargetNone") + delete(declared, int(IndexTargetNone)) + enumerated := make(map[int]struct{}, len(targets)) + names := map[string]bool{} + for _, target := range targets { + _, dup := enumerated[int(target)] + assert.False(t, dup, "duplicate index target %d", target) + enumerated[int(target)] = struct{}{} + assert.NotContains(t, target.String(), "IndexTarget(", "target %d falls through to the numeric name", target) + assert.False(t, names[target.String()], "duplicate index target name %q", target.String()) + names[target.String()] = true + } + assert.Equal(t, declared, enumerated, + "IndexTargets() must enumerate exactly the IndexTarget constants statement.go declares after IndexTargetNone") + assert.Equal(t, "none", IndexTargetNone.String()) +} + // declaredKinds parses statement.go and returns the value of every constant // in the iota block that KindOther opens. func declaredKinds(t *testing.T) map[Kind]struct{} { + t.Helper() + values := declaredIotaValues(t, "KindOther") + declared := make(map[Kind]struct{}, len(values)) + for v := range values { + declared[Kind(v)] = struct{}{} + } + return declared +} + +// declaredIotaValues parses statement.go and returns the value of every +// constant in the iota block that the named anchor constant opens. +func declaredIotaValues(t *testing.T, anchor string) map[int]struct{} { t.Helper() fset := token.NewFileSet() file, err := parser.ParseFile(fset, "statement.go", nil, parser.SkipObjectResolution) @@ -368,25 +405,25 @@ func declaredKinds(t *testing.T) map[Kind]struct{} { continue } first, ok := gen.Specs[0].(*ast.ValueSpec) - if !ok || len(first.Names) != 1 || first.Names[0].Name != "KindOther" { + if !ok || len(first.Names) != 1 || first.Names[0].Name != anchor { continue } - declared := make(map[Kind]struct{}, len(gen.Specs)) + declared := make(map[int]struct{}, len(gen.Specs)) for i, spec := range gen.Specs { vs, ok := spec.(*ast.ValueSpec) require.True(t, ok) - require.Len(t, vs.Names, 1, "one Kind per line in the iota block") + require.Len(t, vs.Names, 1, "one constant per line in the %s iota block", anchor) if i == 0 { - require.Len(t, vs.Values, 1, "KindOther opens the iota block") + require.Len(t, vs.Values, 1, "%s opens the iota block", anchor) ident, ok := vs.Values[0].(*ast.Ident) - require.True(t, ok && ident.Name == "iota", "KindOther is the iota anchor") + require.True(t, ok && ident.Name == "iota", "%s is the iota anchor", anchor) } else { require.Empty(t, vs.Values, "%s takes its value from iota", vs.Names[0].Name) } - declared[Kind(i)] = struct{}{} + declared[i] = struct{}{} } return declared } - t.Fatal("statement.go declares no const block opened by KindOther") + t.Fatalf("statement.go declares no const block opened by %s", anchor) return nil } diff --git a/pkg/verdict/verdict.go b/pkg/verdict/verdict.go index fab574e..12311ee 100644 --- a/pkg/verdict/verdict.go +++ b/pkg/verdict/verdict.go @@ -185,8 +185,23 @@ type Refusal struct { class Class reason Reason owner Owner + cause Cause + site RefusalSite } +// RefusalSite is a typed refusal-site discriminator used when a reason spans +// statement shapes but has no underlying cause. +type RefusalSite string + +const ( + // RefusalSiteIndexSingleRelation is the plain one-relation DROP INDEX, + // REINDEX INDEX, or REINDEX TABLE gate site. + RefusalSiteIndexSingleRelation RefusalSite = "index-statement-single-relation" + // RefusalSiteIndexOther is a multi-relation DROP INDEX or a REINDEX scope + // that does not identify one relation. + RefusalSiteIndexOther RefusalSite = "index-statement-other" +) + // NewRefusal validates and constructs a refusal proof. It rejects a reason // outside Reasons(), a class outside Classes(), an owner outside Owners(), // and an owner that is absent when the class is no-online-safety-problem or @@ -257,19 +272,40 @@ func (r Refusal) Reason() Reason { return r.reason } // no-online-safety-problem. func (r Refusal) Owner() Owner { return r.owner } +// Cause returns the typed cause that narrows the refusal, when one exists. +func (r Refusal) Cause() Cause { return r.cause } + +// Site returns the typed refusal site that narrows the refusal, when one exists. +func (r Refusal) Site() RefusalSite { return r.site } + +// WithCause returns r narrowed by a typed cause. +func (r Refusal) WithCause(cause Cause) Refusal { + r.cause = cause + return r +} + +// WithSite returns r narrowed by a typed refusal site. +func (r Refusal) WithSite(site RefusalSite) Refusal { + r.site = site + return r +} + // IsZero reports whether r was never constructed through NewRefusal. func (r Refusal) IsZero() bool { return r == Refusal{} } // WithRefusal returns v as a refused verdict carrying r's reason, class, -// and owner. It is the one path from a classified refusal onto the verdict -// contract, so a site that forgets to classify has no Reason to set. +// owner, cause, and full in-process proof. It is the one path from a +// classified refusal onto the verdict contract, so a site that forgets to +// classify has no Reason to set. func (v Verdict) WithRefusal(r Refusal) Verdict { - // INV: RF-7 — the verdict's class and owner come from the proof, never - // from the site. + // INV: RF-7, RF-8 — the verdict's refusal fields and in-process proof + // come from the proof, never from the site. v.Outcome = OutcomeRefused v.Reason = r.reason v.Class = r.class v.Owner = r.owner + v.Cause = r.cause + v.proof = r return v } @@ -278,11 +314,59 @@ func (v Verdict) WithRefusal(r Refusal) Verdict { // class and owner through the same one path instead of copying fields. It // fails on a verdict that is not refused, or whose reason, class, and owner // do not validate together — a verdict this build cannot have produced. +// An in-process verdict returns its full proof, re-validated the same way +// and checked against the exported refusal fields, so a proof that never +// passed the constructors, or fields rewritten after WithRefusal, cannot +// reach an eligibility decision. A verdict decoded from JSON reconstructs +// class, reason, owner, and cause, but cannot recover its refusal site; +// site-keyed eligibility therefore fails closed after JSON decoding, while +// cause-keyed eligibility is decidable from the JSON fields. func (v Verdict) Refusal() (Refusal, error) { if v.Outcome != OutcomeRefused { return Refusal{}, fmt.Errorf("verdict outcome is %q, not %q", v.Outcome, OutcomeRefused) } - return NewRefusal(v.Class, v.Reason, v.Owner) + // INV: RF-8 — preserve the full in-process proof; decoded verdicts can + // reconstruct only the refusal fields represented in JSON. + if !v.proof.IsZero() { + return v.provenRefusal() + } + r, err := NewRefusal(v.Class, v.Reason, v.Owner) + if err != nil { + return Refusal{}, err + } + if v.Cause != CauseNone { + r = r.WithCause(v.Cause) + } + return r, nil +} + +// provenRefusal returns the in-process proof once it validates under RF-7 +// and agrees with the verdict's exported refusal fields. Both checks are +// needed: the per-class constructors do not validate their reason, and the +// exported fields are what a consumer reads while the proof is what an +// eligibility decision consumes. +func (v Verdict) provenRefusal() (Refusal, error) { + // INV: RF-7, RF-8 — a proof reaches a consumer only when it is a valid + // classified refusal and the verdict still describes it. + if _, err := NewRefusal(v.proof.class, v.proof.reason, v.proof.owner); err != nil { + return Refusal{}, fmt.Errorf("verdict refusal proof: %w", err) + } + if v.refusalFieldsDivergeFromProof() { + return Refusal{}, fmt.Errorf( + "verdict refusal fields class=%q reason=%q owner=%q cause=%q diverge from proof class=%q reason=%q owner=%q cause=%q", + v.Class, v.Reason, v.Owner, v.Cause, + v.proof.class, v.proof.reason, v.proof.owner, v.proof.cause) + } + return v.proof, nil +} + +// refusalFieldsDivergeFromProof reports whether any exported refusal field +// no longer matches the proof WithRefusal stamped it from. +func (v Verdict) refusalFieldsDivergeFromProof() bool { + return v.Class != v.proof.class || + v.Reason != v.proof.reason || + v.Owner != v.proof.owner || + v.Cause != v.proof.cause } // Cause narrows ReasonBudgetExceeded to the budget that was exceeded, so @@ -299,10 +383,28 @@ const ( // CauseStatementBudget: the statement ran past statement_timeout and was // cancelled; the change needs a rewrite. CauseStatementBudget Cause = "statement-budget" + // CauseParentBlockingIndexBuild identifies a blocking index build on a + // partitioned parent. + CauseParentBlockingIndexBuild Cause = "parent-blocking-index-build" + // CauseParentConcurrentIndexBuild identifies a concurrent index build on + // a partitioned parent. + CauseParentConcurrentIndexBuild Cause = "parent-concurrent-index-build" + // CauseParentIndexAdoption identifies index adoption on a partitioned parent. + CauseParentIndexAdoption Cause = "parent-index-adoption" + // CauseParentNotValidForeignKey identifies a NOT VALID foreign key on a + // partitioned parent. + CauseParentNotValidForeignKey Cause = "parent-not-valid-foreign-key" ) // Verdict is the structured outcome of one migrate invocation. type Verdict struct { + // proof is the full in-process refusal WithRefusal stamped the exported + // fields from. It is deliberately outside the JSON contract: the refusal + // site it carries is not a wire field, so it does not survive decoding, + // and a decoded verdict never compares equal to the in-process verdict it + // was encoded from. Refusal() is the only reader. + proof Refusal + // Outcome is what happened. Outcome Outcome `json:"outcome"` // Reason is the typed refusal cause; empty when executed. diff --git a/pkg/verdict/verdict_test.go b/pkg/verdict/verdict_test.go index b197a2f..69dc3db 100644 --- a/pkg/verdict/verdict_test.go +++ b/pkg/verdict/verdict_test.go @@ -78,11 +78,22 @@ func TestWithRefusalStampsOutcomeReasonClassOwner(t *testing.T) { } func TestRefusalRoundTripsThroughVerdict(t *testing.T) { - want := NoOnlineSafetyProblem(ReasonUnsupportedStatement, OwnerProvisioning) + want := ByDesign(ReasonIndexStatement). + WithCause(CauseStatementBudget). + WithSite(RefusalSiteIndexSingleRelation) got, err := Verdict{Statement: "GRANT SELECT ON t TO r"}.WithRefusal(want).Refusal() require.NoError(t, err) assert.Equal(t, want, got) + encoded, err := json.Marshal(Verdict{}.WithRefusal(want)) + require.NoError(t, err) + var decoded Verdict + require.NoError(t, json.Unmarshal(encoded, &decoded)) + got, err = decoded.Refusal() + require.NoError(t, err) + assert.Equal(t, CauseStatementBudget, got.Cause()) + assert.Equal(t, RefusalSite(""), got.Site()) + _, err = Verdict{Outcome: OutcomeExecuted}.Refusal() require.Error(t, err, "an executed verdict carries no refusal") @@ -93,6 +104,56 @@ func TestRefusalRoundTripsThroughVerdict(t *testing.T) { require.Error(t, err, "an owner outside no-online-safety-problem violates RF-7") } +// The in-process fast path validates the proof the same way the decoded +// path validates the JSON fields: a non-zero proof that never passed the +// constructors is rejected rather than returned as-is. +func TestRefusalRejectsUnvalidatedProof(t *testing.T) { + tests := []struct { + name string + proof Refusal + }{ + {"site on the zero refusal", Refusal{}.WithSite(RefusalSiteIndexSingleRelation)}, + {"cause on the zero refusal", Refusal{}.WithCause(CauseLockBudget)}, + {"reason outside Reasons()", ByDesign("brand-new-reason")}, + {"owner on a class that carries none", Refusal{class: ClassByDesign, reason: ReasonIndexStatement, owner: OwnerDirectOperator}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.False(t, tc.proof.IsZero(), "the fixture must take the in-process path") + _, err := Verdict{}.WithRefusal(tc.proof).Refusal() + assert.Error(t, err) + }) + } +} + +// The exported refusal fields are what a consumer reads; the proof is what +// an eligibility decision consumes. A verdict whose fields were rewritten +// after WithRefusal describes a different refusal than it proves, and +// Refusal() refuses to hand out either. +func TestRefusalRejectsFieldsDivergingFromProof(t *testing.T) { + proof := CapabilityBoundary(ReasonUnsupportedPartitionedParent).WithCause(CauseParentBlockingIndexBuild) + tests := []struct { + name string + mutate func(*Verdict) + }{ + {"class", func(v *Verdict) { v.Class = ClassByDesign }}, + {"reason", func(v *Verdict) { v.Reason = ReasonIndexStatement }}, + {"owner", func(v *Verdict) { v.Owner = OwnerDirectOperator }}, + {"cause", func(v *Verdict) { v.Cause = CauseParentConcurrentIndexBuild }}, + {"cause cleared", func(v *Verdict) { v.Cause = CauseNone }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + v := Verdict{}.WithRefusal(proof) + _, err := v.Refusal() + require.NoError(t, err, "the unmodified verdict agrees with its proof") + tc.mutate(&v) + _, err = v.Refusal() + assert.ErrorContains(t, err, "diverge from proof") + }) + } +} + func TestJSONRoundTrip(t *testing.T) { v := Verdict{ Outcome: OutcomeRefused, @@ -163,7 +224,11 @@ func TestJSONOmitsEmptyOptionalFields(t *testing.T) { // Reason and Cause values are the machine contract automation switches on: // flat kebab-case tokens, no spaces or colons — prose belongs in Detail. func TestReasonAndCauseTokensAreFlat(t *testing.T) { - toks := []string{string(CauseLockBudget), string(CauseStatementBudget)} + toks := []string{ + string(CauseLockBudget), string(CauseStatementBudget), + string(CauseParentBlockingIndexBuild), string(CauseParentConcurrentIndexBuild), + string(CauseParentIndexAdoption), string(CauseParentNotValidForeignKey), + } for _, r := range Reasons() { toks = append(toks, string(r)) }