diff --git a/README.md b/README.md index 6cd262dc..d7893a95 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,8 @@ refusal — never a silently wrong or incomplete result: which keeps its values. A `DROP INDEX` run under `--accept-blocking` needs `--allow-destructive` too: accepting the lock is not accepting the loss, and the refusal lists the checks (`idx_scan` on every replica, foreign - keys, uniqueness) that catch an index the application still uses. + keys, uniqueness) that catch an index the application still uses, with + the connected server's own scan count for the index in `index_usage`. - **Destructive desired-state plans are refused as a whole** — `migrate --desired` creates missing tables and converges supported changes, but never infers permission to discard live structure. A new primary key or unique constraint over a live diff --git a/SAFETY.md b/SAFETY.md index f707b186..9d594b52 100644 --- a/SAFETY.md +++ b/SAFETY.md @@ -18,7 +18,7 @@ The invariant registry (invariant IDs referenced below) lives in | Package | Core? | Status | Invariants enforced | | --- | --- | --- | --- | | `pkg/dbconn` — pool defaults, terminate-blockers, retries, RDS TLS, advisory table lock, replication connection | ✅ core | table-lock primitive exists; `ConnectReplication` dials one replication-mode connection with the pool's TLS, timeouts, and `BeforeConnect` hook; wiring into executing modes planned | LK-1 primitive; LK-2 primitives; CO-9 (session hook and `LocalSearchPath`) | -| `pkg/preflight` — precondition verifier, refusals | ✅ core | exists; copy-and-swap target proof (shape and dependents) and cluster/volume environment check gate the copy-and-swap route before any shadow object is created; table usage evidence (`ReadTableUsage`, `TableUsage.Judge`) exists with no caller yet | ST-6, RF-1..RF-5, RF-11, CO-9 (every proof read is `pg_catalog`-qualified) | +| `pkg/preflight` — precondition verifier, refusals | ✅ core | exists; copy-and-swap target proof (shape and dependents) and cluster/volume environment check gate the copy-and-swap route before any shadow object is created; table usage evidence (`ReadTableUsage`, `TableUsage.Judge`) exists with no caller yet; index usage evidence (`ReadIndexUsage`) is reported on a refused `DROP INDEX` | ST-6, RF-1..RF-5, RF-11, CO-9 (every proof read is `pg_catalog`-qualified) | | `pkg/executor` — bounded optimistic attempt; native concurrent index build with invalid-index recovery; native sequence executor for the safer idioms | ✅ core | exists (Phase 1: attempt-under-budget; Phase 3.1: concurrent index build; Phase 3.2: sequence executor) | LK-2 (attempt bound + the CONCURRENTLY wait-policy exception), CO-9 (qualified proof reads), ST-9 (create owner verified, never repaired) | | `pkg/checksum` — chunk verifier, divergence policy, repair, continuous checker | ✅ core | the chunk `Verifier` exists (one read-only `REPEATABLE READ` transaction per chunk in which the chunk is cut and both digests read, so all three share a snapshot and the cut is bounded, every column cast to the shadow's type on both sides, the same guard as the copier — owner role, the copy statement's `search_path` (the catalog alone, or the shadow build's path when the proof carries conversions), `ACCESS SHARE` on both relations, lock confirmation, relation-OID check — and a `Report` of mismatched chunks up to the landed watermark); `Check` runs the pass under an explicit `DivergencePolicy` (zero value refused; `ParseDivergencePolicy` for configuration, which names `abort` or `repair` and refuses `recopy`), aborts with the shadow untouched or recopies every differing chunk with the copier's chunk statement in one guarded transaction — every chunk's rows deleted before any are put back, so a unique value the source moved between two chunks lands — then rereads each chunk under `repair`, or commits without the reread under `recopy` (the reconcile mode's policy, for a source written while the pass runs; a recopy that collides with a shadow row because the source moved a unique value from a clean chunk into a differing one is rolled back and the pass compares and recopies again, a bounded number of times, counting each collision), returning the committed repairs with any refusal, and mints `CleanWatermark` and `VerifiedShadow` (private constructors) only from a pass that found nothing and repaired nothing; `CheckLive` is the pass for a source written while the catch-up applies the stream — a differing chunk is reread once the catch-up has confirmed past the insert position it was read at and judged against the earlier read (settled, hot and reread again up to a bound, or a confirmed divergence), an unsettled chunk is an `UnresolvedError` that mints nothing, a live repair holds the catch-up's flushes off the shadow in Go for as long as it recopies (`LiveOptions.Flushes`, a `FlushGate` the applier's `Catchup` satisfies, required under `repair`) with `SHARE MODE` on the shadow against every other writer, resumes them once the recopy has committed, and settles each repaired chunk the same way, and `recopy` is refused; the pass is the progress tracker's `WorkSource` while it runs (chunks compared, rows hashed, chunks mismatched, chunks repaired, chunks reread, all from memory); the continuous checker that runs `CheckLive` on a loop is planned | CO-1, CO-2, CO-3, CO-9, LK-1, LK-2 | | `pkg/copier` — shadow-table chunked copy | ✅ core | contract types and the keyset `Chunker` exist (row-count chunks over the proven key, first chunk open below and last open above, a cut frontier for the applier's discard rule, time-targeted sizing) and the parallel `Copier` (one bounded never-overwriting insert per chunk under the table lock session — each column the proof's conversions name selected as its `USING` expression, every other bare — each in a guarded transaction — owner role, the copy statement's `search_path` (the catalog alone, or the shadow build's path when the copy carries conversions), `ACCESS SHARE` on both relations, lock confirmation, relation-OID check — frontier-ordered in-flight registry, a resume that first clears the shadow above the watermark, `Position.Classify` for the applier, and the copy step's `progress.WorkSource` — rows from committed chunks, the source's catalog row count, both tables' measured sizes; every cut and measurement in a bounded read-only transaction with the catalog alone on its `search_path`) exist; every refusal is a `*RefusalError` carrying a typed `RefusalCause` from the closed set in [docs/refusal-classes.md](docs/refusal-classes.md#copier-refusals-keyed-on-copierrefusalcause) | CO-4 (chunk coverage, copy SQL shape, in-flight registry), CO-9, LK-1, LK-2 (every statement on the caller's pool bounds itself), LK-3 | diff --git a/demo/tour.sh b/demo/tour.sh index 43d6d91e..26a8df66 100755 --- a/demo/tour.sh +++ b/demo/tour.sh @@ -246,6 +246,7 @@ execute_accepted_needs_loss() { assert_eq "refusal exit of [$sql]" 2 "$status" assert_eq "reason of [$sql]" "destructive-change" "$(jq -r '.reason' <<<"$out")" assert_eq "table of [$sql]" "$table" "$(jq -r '.table' <<<"$out")" + assert_eq "index_usage.idx_scan is a number in [$sql]" number "$(jq -r '.index_usage.idx_scan | type' <<<"$out")" else "$PGS" migrate --url "$PG_DSN" --alter "$sql" --accept-blocking "$table" --statement-timeout 30s \ || echo "(refused, exit $? — expected 2: a dropped index also needs --allow-destructive)" diff --git a/docs/invariants.md b/docs/invariants.md index 2272ebef..1c4c9c8f 100644 --- a/docs/invariants.md +++ b/docs/invariants.md @@ -1413,7 +1413,8 @@ Each refusal is a preflight **error with a stated reason** — never a warning, reason. A `DROP INDEX` — unique or not — is gate-refused first; when `--accept-blocking` accepts that refusal, the drop still runs only if `--allow-destructive` names the same owning table, and otherwise it is refused with `destructive-change` before anything - executes, the detail listing the checks that catch an index the application still uses. + executes, the detail listing the checks that catch an index the application still uses and + the verdict's `index_usage` carrying the connected server's live evidence for the index. Accepting the lock is not accepting the loss. Outage-risk statements are covered by RF-5 and AB-1..AB-5 (`--force`, `--accept-blocking`) and by the LK-2 budgets. *Enforced:* `pkg/migrate` (`run`'s destructive gate through `destructiveOperations` and @@ -1429,7 +1430,10 @@ Each refusal is a preflight **error with a stated reason** — never a warning, `TestRunRefusesAnUnacknowledgedDropColumnRoutedToCopyAndSwap`, `TestRunNeedsBothForceAndTheDestructiveAcknowledgement`, `TestRunAllowDestructiveDoesNotLiftADropIndexRefusal`, - `TestRunAcceptBlockingNeedsTheDestructiveAcknowledgementToDropAnIndex`, `TestAdmitDestructive`, + `TestRunAcceptBlockingNeedsTheDestructiveAcknowledgementToDropAnIndex`, + `TestReadIndexUsageCountsEachIndexsOwnScans`, + `TestReadIndexUsageSumsAPartitionedIndexsLeafIndexes`, + `TestCheckLeafStatisticsListedRefusesAnUnlistedLeaf`, `TestAdmitDestructive`, `TestMigrateDryRunReportsTheDestructiveRefusal`, `TestAllowDestructiveFlagGrammar`, `demo/tour.sh` (`refused_destructive`). *Source:* [design principles](design-principles.md#declarative-review-first-workflow) diff --git a/docs/limitations.md b/docs/limitations.md index 5a351f1f..1414af1f 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -15,7 +15,7 @@ not escape hatches: | `ADD CONSTRAINT ... USING INDEX` on a partitioned parent | PostgreSQL does not support adopting an existing index on a partitioned parent in any supported version. pg-sprite refuses before execution. | | `ADD PRIMARY KEY` / `ADD UNIQUE` with a storage parameter the online sequence cannot render | The online sequence builds the key's index with `CREATE UNIQUE INDEX CONCURRENTLY` and carries the key's `WITH (...)` parameters onto it, each name quoted and each value a quoted literal. A parameter it cannot render faithfully, one whose value is not a plain word, number, or string, contains a backslash, or whose name is namespaced, would be dropped or changed, so no sequence is built. Running the statement as written builds the index under `ACCESS EXCLUSIVE`, so it is refused as `rewrite-required` with the `unique-index-then-constraint` guidance. Build the index concurrently with the parameter, then attach it with `ADD CONSTRAINT ... USING INDEX`. | | `ADD FOREIGN KEY ... NOT VALID` on a partitioned parent | PostgreSQL does not support this before version 18, so pg-sprite refuses it on versions 14–17. It is supported on version 18 and later. | -| `DROP INDEX` and `REINDEX`, plain or `CONCURRENTLY` | `migrate` never runs index maintenance online-safely. The plain form is refused with `index-statement` and names its `CONCURRENTLY` idiom; the concurrent form is refused as already safe, so run it directly against the database. `migrate --accept-blocking SCHEMA.TABLE` runs a single-relation plain `DROP INDEX`, `REINDEX INDEX`, or `REINDEX TABLE` as-is under bounded budgets and exits 3 ([lock-budgeted-passthrough.md](lock-budgeted-passthrough.md)). A `DROP INDEX`, unique or not, also needs `--allow-destructive` naming the same table: without it the drop is refused with `destructive-change` (exit 2) before anything runs, and the detail lists the checks to make first — `idx_scan` is 0 on the primary and every replica over a full application cycle, no foreign key relies on the index, and nothing needs a unique index's guarantee. `diff`'s hint to run a removed index through `migrate --alter` ends in this refusal; run `DROP INDEX CONCURRENTLY` directly. | +| `DROP INDEX` and `REINDEX`, plain or `CONCURRENTLY` | `migrate` never runs index maintenance online-safely. The plain form is refused with `index-statement` and names its `CONCURRENTLY` idiom; the concurrent form is refused as already safe, so run it directly against the database. `migrate --accept-blocking SCHEMA.TABLE` runs a single-relation plain `DROP INDEX`, `REINDEX INDEX`, or `REINDEX TABLE` as-is under bounded budgets and exits 3 ([lock-budgeted-passthrough.md](lock-budgeted-passthrough.md)). A `DROP INDEX`, unique or not, also needs `--allow-destructive` naming the same table: without it the drop is refused with `destructive-change` (exit 2) before anything runs, and the detail lists the checks to make first — `idx_scan` is 0 on the primary and every replica over a full application cycle, no foreign key relies on the index, and nothing needs a unique index's guarantee. The refusal's `index_usage` carries the connected server's own evidence — `idx_scan`, `stats_reset`, `last_idx_scan` (PostgreSQL 16+), uniqueness, and whether `track_counts` was on — which covers that one server only, never its replicas. `diff`'s hint to run a removed index through `migrate --alter` ends in this refusal; run `DROP INDEX CONCURRENTLY` directly. | | `RENAME COLUMN` and `RENAME TO` | Without `--force`, refused with `not-native-safe-rewrite-required`, nothing executed: the catalog flip is instant, but every running application instance that still queries the old name breaks the moment it commits, and no safer sequence can be built from the statement alone because the dual-write and the read switch live in the application. The plan report, `suggest`, and the verdict carry the manual path as typed guidance — `expand-contract-column` for a column (add the new column, dual-write and backfill, switch reads, drop the old column as its own reviewed change) and `coordinated-table-rename` for a table (coordinate the rename with the deploy that adopts the new name, or keep the old name readable through a view until every instance has moved) — and `lint` reports the rename as an error. `--force SCHEMA.TABLE` runs the submitted rename as a blind bounded attempt under the usual budgets and size guard, for the operator who has coordinated the deploy; on a table above `--max-table-size` (default 1 GiB) the forced rename is refused as `not-native-safe-table-too-large` until that threshold is raised. | | `ALTER INDEX ... RENAME TO` on an index that backs a primary-key, unique, or exclusion constraint | Refused with `not-native-safe-rewrite-required`, class `by-design`, guidance `coordinated-constraint-rename`, nothing executed: PostgreSQL renames the constraint with its index, and `ON CONFLICT ON CONSTRAINT` and error handling that name the old constraint break the moment it commits. `--force` does not apply to an index rename. A plain index rename runs. The refusal needs the catalog, so offline `lint` and `suggest` do not report it. | | `ATTACH PARTITION` | Without `--force`, refused with `not-native-safe-rewrite-required`, nothing executed: the engine does not yet construct the validated `CHECK` that lets the attach skip its scan. Pre-prove the bound yourself, then run the attach directly. `--force SCHEMA.TABLE` runs the blocking `ATTACH PARTITION` as submitted under bounded budgets, taking the blocking lock. | diff --git a/docs/lock-budgeted-passthrough.md b/docs/lock-budgeted-passthrough.md index 387a71a8..c9dbdbd1 100644 --- a/docs/lock-budgeted-passthrough.md +++ b/docs/lock-budgeted-passthrough.md @@ -142,7 +142,16 @@ A `DROP INDEX` needs both acknowledgements, each naming the owning table: `--acc accepts the lock, and `--allow-destructive` accepts losing the index (RF-10). The second is checked after the owning table resolves and before anything executes; without it the drop is refused with `destructive-change`, and the detail lists the checks that catch an index the -application still uses. `REINDEX INDEX` and `REINDEX TABLE` discard nothing and need only +application still uses. The refusal also carries `index_usage`, the connected server's live +evidence for the index (`preflight.ReadIndexUsage`, keyed on the index OID the owning-table +lookup resolved): its `idx_scan` since `stats_reset` (or since the index was created, if +later, which the server does not record), `last_idx_scan` on PostgreSQL 16 and later, whether +it is unique, whether `track_counts` was on, and whether a saved role or database setting +turns it off. A partitioned index has no statistics of its own, so its `idx_scan` is the sum +over its leaf indexes and `partitions` counts them. It is one server's view — +replicas keep their own counters — so it is evidence for the operator, not a gate. Its times +are UTC. The read takes no lock; if it fails, including when the server lists no statistics +for a leaf index, nothing executes and the error is returned instead of the refusal. `REINDEX INDEX` and `REINDEX TABLE` discard nothing and need only `--accept-blocking`. The value names the table whose lock the operator accepts, because that is the relation the diff --git a/docs/schemabot-integration.md b/docs/schemabot-integration.md index 89eded40..22bec4f1 100644 --- a/docs/schemabot-integration.md +++ b/docs/schemabot-integration.md @@ -634,7 +634,13 @@ and PostgreSQL `lock_timeout`). Contract points for the PostgreSQL adapter: executed. An acknowledged run's verdict carries `allowed_destructive: true`, the machine-readable audit record to store beside the operator's consent. The acknowledgement covers only the loss: a statement that also needs `Force` still needs it, and a `DROP INDEX` - run with `Options.AcceptBlocking` needs both, each naming the index's owning table. + run with `Options.AcceptBlocking` needs both, each naming the index's owning table; its + refusal carries `index_usage` (the writer's `idx_scan`, summed over the leaf indexes of a + partitioned index with `partitions` counting them, `stats_reset`, `last_idx_scan`, + uniqueness, `track_counts`, `track_counts_off_by_setting`; times in UTC) to show the + operator before asking for that consent. If that read fails, `Run` returns the error with a + zero verdict and nothing executed rather than the refusal: route it as a retryable + failure, like a connection fault, not as a refusal. `RunDesired` rejects `AllowDestructive` (`ErrAllowDestructiveNotSupported`) and keeps refusing a destructive plan whole, so a declarative drop the operator approves runs as its own diff --git a/docs/table-usage.md b/docs/table-usage.md index d5c4aa83..6ae28248 100644 --- a/docs/table-usage.md +++ b/docs/table-usage.md @@ -8,7 +8,11 @@ those statistics can prove. This page explains what the check reads, how it deci `DROP TABLE` is still refused by both front doors today ([capabilities](capabilities.md#deliberately-operator-owned)). The check described here is a read-only primitive, `preflight.ReadTableUsage` and `TableUsage.Judge`, that a later -drop path will consult. Nothing calls it yet. +drop path will consult. Nothing calls it yet. Its index-level sibling, +`preflight.ReadIndexUsage`, reads one index's `idx_scan`, `last_idx_scan`, and uniqueness +(summed over the leaf indexes of a partitioned index) under the same limits, including the +saved `track_counts` settings check, and is reported, never judged, on a refused `DROP INDEX` +([lock-budgeted passthrough](lock-budgeted-passthrough.md#flag-and-front-door)). ## Read this first: what the check can see diff --git a/docs/testing.md b/docs/testing.md index cf54d89f..a91f2ee2 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -409,6 +409,7 @@ same package cover the rest of it. | Library front door (`diffplan.Plan`): ordered routed plan, missing-table, no-op, copy-and-swap refusal, never-writes, deterministic fingerprint | [diffplan unit](../pkg/diffplan/diffplan_test.go), [diffplan integration](../pkg/diffplan/diffplan_integration_test.go) | | Bounded optimistic native attempt and table preflight | [pkg/executor](../pkg/executor/optimistic_integration_test.go), [pkg/preflight](../pkg/preflight/preflight_integration_test.go) | | Table usage evidence (`ReadTableUsage`, `Judge`): window edges, recent scans and lock holders, PostgreSQL 14/15 unknown, resets never idle, one statistics snapshot per read, `track_counts` turned off by a saved setting, partitioned tables judged across every leaf partition | [pkg/preflight](../pkg/preflight/table_usage_integration_test.go), [coverage](../pkg/preflight/table_usage_coverage_integration_test.go), [partitions](../pkg/preflight/table_usage_partitions_integration_test.go), [verdict (unit)](../pkg/preflight/table_usage_verdict_test.go), [partition aggregation (unit)](../pkg/preflight/table_usage_partitions_test.go) | +| Index usage evidence (`ReadIndexUsage`): each index's own scan count, uniqueness, last scan time on PostgreSQL 16+, a partitioned index summed over its leaf indexes at any depth, `track_counts` turned off by a saved setting, a non-index OID refused | [pkg/preflight](../pkg/preflight/index_usage_integration_test.go), [partitions](../pkg/preflight/index_usage_partitions_integration_test.go), [evidence and detail (unit)](../pkg/migrate/index_usage_evidence_test.go) | | Safer-sequence runner: autocommit steps, committed-prefix semantics on a mid-sequence failure | [pkg/executor](../pkg/executor/sequence_integration_test.go) | | Concurrent index build: caller-owned session, invalid-index detection into a typed state, and the proven recovery (`RebuildAbandonedIndex`, `DropAbandonedIndex`) | [pkg/executor](../pkg/executor/recover_integration_test.go) | | Greenfield `CREATE TABLE` path: shape refusals, claimed-name collisions, post-commit name verification | [pkg/executor](../pkg/executor/create_integration_test.go) | diff --git a/internal/cli/color_test.go b/internal/cli/color_test.go index 7f95c80e..f6fc3f28 100644 --- a/internal/cli/color_test.go +++ b/internal/cli/color_test.go @@ -187,6 +187,7 @@ func fullVerdict(t *testing.T) verdict.Verdict { LockTimeout: "3s", StatementTimeout: "10m", StatisticsRefresh: &verdict.StatisticsRefresh{Columns: []string{"c"}, Pending: `ANALYZE "t"`}, + IndexUsage: &verdict.IndexUsage{IdxScans: 7, Unique: true, TrackCounts: true}, } rv := reflect.ValueOf(v) for i := range rv.NumField() { diff --git a/internal/cli/verdict_text.go b/internal/cli/verdict_text.go index 81e2928c..648df513 100644 --- a/internal/cli/verdict_text.go +++ b/internal/cli/verdict_text.go @@ -51,6 +51,9 @@ func writeVerdictText(out io.Writer, pal palette, v verdict.Verdict) error { if v.WritesBlocked { fmt.Fprintf(&b, "\n %s writes to the table were refused (55006) while the run held it", pal.bold("blocked:")) } + if v.IndexUsage != nil { + fmt.Fprintf(&b, "\n %s %s", pal.bold("usage:"), v.IndexUsage.Summary()) + } if v.AllowedDestructive { fmt.Fprintf(&b, "\n %s the statement's data loss was acknowledged (allow-destructive)", pal.bold("allowed:")) } diff --git a/pkg/migrate/accept_blocking.go b/pkg/migrate/accept_blocking.go index ad1883be..4f4fdcb9 100644 --- a/pkg/migrate/accept_blocking.go +++ b/pkg/migrate/accept_blocking.go @@ -11,6 +11,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "github.com/block/pg-sprite/pkg/executor" + "github.com/block/pg-sprite/pkg/preflight" "github.com/block/pg-sprite/pkg/statement" "github.com/block/pg-sprite/pkg/verdict" ) @@ -82,7 +83,11 @@ func acceptBlocking(ctx context.Context, pool *pgxpool.Pool, st statement.Statem dropsIndex := st.Kind() == statement.KindDropIndex if dropsIndex { if opts.AllowDestructive == "" { - return dropIndexDestructiveVerdict(st, table), nil + usage, err := preflight.ReadIndexUsage(ctx, pool, locked.relationOID) + if err != nil { + return verdict.Verdict{}, fmt.Errorf("read usage of the index to drop on %s: %w; nothing was executed", table, err) + } + return dropIndexDestructiveVerdict(st, table, usage), nil } auditAllowDestructive(opts.audit(), st, table) } @@ -154,10 +159,12 @@ func acceptBlocking(ctx context.Context, pool *pgxpool.Pool, st statement.Statem } // acceptedTable is the table an accepted statement locks, as the catalog -// resolved it, with the relkind of that table. +// resolved it, with the relkind of that table and the OID of the relation +// the statement names (the index, or the table itself). type acceptedTable struct { schema, name string relkind string + relationOID uint32 } // qualified renders the table as the acknowledgement must spell it. @@ -198,7 +205,7 @@ func lockedTable(ctx context.Context, pool *pgxpool.Pool, rel statement.IndexRel // against. to_regclass itself still resolves the bare name on that path, // exactly as the statement will. const q = ` - SELECT c.relkind::pg_catalog.text, + SELECT c.oid, c.relkind::pg_catalog.text, COALESCE(tn.nspname, n.nspname), COALESCE(t.relname, c.relname), COALESCE(t.relkind, c.relkind)::pg_catalog.text @@ -211,7 +218,7 @@ func lockedTable(ctx context.Context, pool *pgxpool.Pool, rel statement.IndexRel name := regclassName(rel) var relkind string var table acceptedTable - err := pool.QueryRow(ctx, q, name).Scan(&relkind, &table.schema, &table.name, &table.relkind) + err := pool.QueryRow(ctx, q, name).Scan(&table.relationOID, &relkind, &table.schema, &table.name, &table.relkind) if errors.Is(err, pgx.ErrNoRows) { return acceptedTable{}, fmt.Errorf("%w: %s is not visible on the session search_path; nothing was executed", ErrAcceptBlockingRelationNotFound, name) diff --git a/pkg/migrate/accept_blocking_integration_test.go b/pkg/migrate/accept_blocking_integration_test.go index 7664734b..a7b1918d 100644 --- a/pkg/migrate/accept_blocking_integration_test.go +++ b/pkg/migrate/accept_blocking_integration_test.go @@ -268,6 +268,16 @@ func TestRunAcceptBlockingNeedsTheDestructiveAcknowledgementToDropAnIndex(t *tes assert.False(t, v.AllowedDestructive) assert.Contains(t, v.Detail, "idx_scan") assert.Contains(t, v.Detail, "--allow-destructive "+schema+".orders") + // The refusal carries this server's live evidence for the index it + // refused to drop: a fresh, non-unique index nothing has scanned. + require.NotNil(t, v.IndexUsage) + assert.EqualValues(t, 0, v.IndexUsage.IdxScans) + assert.False(t, v.IndexUsage.Unique) + assert.True(t, v.IndexUsage.TrackCounts) + assert.False(t, v.IndexUsage.TrackCountsOffBySetting) + assert.Nil(t, v.IndexUsage.Partitions, "a plain index has no leaf count") + assert.False(t, v.IndexUsage.ReadAt.IsZero()) + assert.Equal(t, time.UTC, v.IndexUsage.ReadAt.Location()) assert.True(t, indexExists(t, pool, schema, "orders_created_idx"), "the refused drop must execute nothing") }) diff --git a/pkg/migrate/allow_destructive.go b/pkg/migrate/allow_destructive.go index 9edef635..0b50ad07 100644 --- a/pkg/migrate/allow_destructive.go +++ b/pkg/migrate/allow_destructive.go @@ -5,7 +5,9 @@ import ( "fmt" "log/slog" "strings" + "time" + "github.com/block/pg-sprite/pkg/preflight" "github.com/block/pg-sprite/pkg/router" "github.com/block/pg-sprite/pkg/statement" "github.com/block/pg-sprite/pkg/suggest" @@ -132,19 +134,83 @@ func AdmitDestructive(st statement.Statement, schema string, rs router.Statement // INDEX without the destructive acknowledgement. Every index drop counts, // not only a unique one: the common mistake is dropping an index the // application still reads through, so the detail lists the checks that -// catch it before naming the acknowledgement. table is the index's owning -// table, resolved from the catalog. -func dropIndexDestructiveVerdict(st statement.Statement, table string) verdict.Verdict { +// catch it before naming the acknowledgement, and carries this server's +// live usage of the index as the first piece of that evidence. table is the +// index's owning table, resolved from the catalog. +func dropIndexDestructiveVerdict(st statement.Statement, table string, usage preflight.IndexUsage) verdict.Verdict { detail := "DROP INDEX discards an index that queries, foreign-key checks, or a uniqueness guarantee may " + - "still rely on, and undoing it is a full index build; before dropping, confirm that " + - "pg_stat_user_indexes.idx_scan for the index is 0 on the primary and on every replica (each instance " + - "keeps its own counters) over a window since stats_reset that covers the application's full cycle, " + - "that no foreign key's referencing columns rely on it, and, for a unique index, that nothing still " + - "needs the uniqueness it enforces — then rerun with --allow-destructive " + table - v := verdict.Verdict{Statement: st.SQL(), Table: table, Detail: detail} + "still rely on, and undoing it is a full index build; " + describeIndexUsage(usage) + + "; before dropping, confirm that pg_stat_user_indexes.idx_scan for the index is 0 on every replica " + + "too (each instance keeps its own counters) over a window since stats_reset that covers the " + + "application's full cycle, that no foreign key's referencing columns rely on it, and, for a unique " + + "index, that nothing still needs the uniqueness it enforces — then rerun with --allow-destructive " + table + v := verdict.Verdict{ + Statement: st.SQL(), + Table: table, + Detail: detail, + IndexUsage: indexUsageEvidence(usage), + } return v.WithRefusal(destructiveChangeRefusal()) } +// indexUsageEvidence copies the read into the verdict with every instant in +// UTC, so the JSON is the same whichever host's time zone ran pg-sprite. +func indexUsageEvidence(u preflight.IndexUsage) *verdict.IndexUsage { + e := &verdict.IndexUsage{ + IdxScans: u.IdxScans, + LastIdxScan: utcTime(u.LastIdxScan), + StatsReset: utcTime(u.StatsReset), + ReadAt: u.ReadAt.UTC(), + Unique: u.Unique, + TrackCounts: u.TrackCounts, + TrackCountsOffBySetting: u.TrackCountsOffBySetting, + } + if u.Partitioned { + partitions := u.Partitions + e.Partitions = &partitions + } + return e +} + +func utcTime(t *time.Time) *time.Time { + if t == nil { + return nil + } + utc := t.UTC() + return &utc +} + +// describeIndexUsage renders the connected server's evidence for the +// refusal detail: the scan count, since when it counts, the last scan when +// the server records one, and the caveats that make a zero meaningless. +// The server does not record when an index was created, and an index +// counts only from then, so the window is stated as the later of the two. +func describeIndexUsage(u preflight.IndexUsage) string { + since := "since its statistics began (no reset recorded)" + if u.StatsReset != nil { + since = "since stats_reset at " + u.StatsReset.UTC().Format(time.RFC3339) + } + since += " or since the index was created, if later (the server does not record when)" + object := "the index" + if u.Partitioned { + object = fmt.Sprintf("the index through its partition indexes (%d)", u.Partitions) + } + s := fmt.Sprintf("this server has scanned %s %d times %s", object, u.IdxScans, since) + if u.LastIdxScan != nil { + s += ", last at " + u.LastIdxScan.UTC().Format(time.RFC3339) + } + if u.Unique { + s += "; it is a unique index" + } + if !u.TrackCounts { + s += "; track_counts is off, so this server counted no scans and the count proves nothing" + } + if u.TrackCountsOffBySetting { + s += "; an ALTER ROLE or ALTER DATABASE setting turns track_counts off, so scans by those sessions are not counted" + } + return s +} + // auditAllowDestructive records the acknowledged data loss on table before // anything executes. The record is warn-level and unconditional, as the // force audit is, and the verdict's AllowedDestructive field is its diff --git a/pkg/migrate/index_usage_evidence_test.go b/pkg/migrate/index_usage_evidence_test.go new file mode 100644 index 00000000..c4fea3fe --- /dev/null +++ b/pkg/migrate/index_usage_evidence_test.go @@ -0,0 +1,91 @@ +package migrate + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/preflight" +) + +// The evidence's instants are UTC whatever zone pgx scanned them in, so the +// verdict's JSON does not depend on the host's time zone, and a partitioned +// index carries its leaf count while a plain one carries none. +func TestIndexUsageEvidenceIsUTCAndCountsPartitions(t *testing.T) { + east := time.FixedZone("UTC+8", 8*60*60) + readAt := time.Date(2026, 11, 2, 11, 13, 17, 0, east) + reset := readAt.Add(-48 * time.Hour) + last := readAt.Add(-time.Hour) + + e := indexUsageEvidence(preflight.IndexUsage{ + IdxScans: 5, ReadAt: readAt, StatsReset: &reset, LastIdxScan: &last, + Partitioned: true, Partitions: 2, TrackCounts: true, TrackCountsOffBySetting: true, + }) + + assert.Equal(t, time.UTC, e.ReadAt.Location()) + assert.True(t, e.ReadAt.Equal(readAt)) + require.NotNil(t, e.StatsReset) + assert.Equal(t, time.UTC, e.StatsReset.Location()) + require.NotNil(t, e.LastIdxScan) + assert.Equal(t, time.UTC, e.LastIdxScan.Location()) + require.NotNil(t, e.Partitions) + assert.Equal(t, 2, *e.Partitions) + assert.EqualValues(t, 5, e.IdxScans) + assert.True(t, e.TrackCountsOffBySetting) + + plain := indexUsageEvidence(preflight.IndexUsage{ReadAt: readAt, TrackCounts: true}) + assert.Nil(t, plain.Partitions) + assert.Nil(t, plain.StatsReset) + assert.Nil(t, plain.LastIdxScan) +} + +// describeIndexUsage is the detail's renderer: each piece of evidence and +// each caveat appears exactly when its field says so. +func TestDescribeIndexUsage(t *testing.T) { + reset := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC) + last := time.Date(2026, 11, 1, 8, 30, 0, 0, time.UTC) + const created = " or since the index was created, if later (the server does not record when)" + + cases := []struct { + name string + usage preflight.IndexUsage + want string + }{ + { + name: "no reset recorded", + usage: preflight.IndexUsage{TrackCounts: true}, + want: "this server has scanned the index 0 times since its statistics began (no reset recorded)" + created, + }, + { + name: "reset and last scan", + usage: preflight.IndexUsage{IdxScans: 7, StatsReset: &reset, LastIdxScan: &last, TrackCounts: true}, + want: "this server has scanned the index 7 times since stats_reset at 2026-09-01T00:00:00Z" + created + + ", last at 2026-11-01T08:30:00Z", + }, + { + name: "partitioned and unique", + usage: preflight.IndexUsage{IdxScans: 5, Partitioned: true, Partitions: 2, Unique: true, TrackCounts: true}, + want: "this server has scanned the index through its partition indexes (2) 5 times since its statistics began (no reset recorded)" + + created + "; it is a unique index", + }, + { + name: "track_counts off", + usage: preflight.IndexUsage{}, + want: "this server has scanned the index 0 times since its statistics began (no reset recorded)" + created + + "; track_counts is off, so this server counted no scans and the count proves nothing", + }, + { + name: "track_counts off by a saved setting", + usage: preflight.IndexUsage{TrackCounts: true, TrackCountsOffBySetting: true}, + want: "this server has scanned the index 0 times since its statistics began (no reset recorded)" + created + + "; an ALTER ROLE or ALTER DATABASE setting turns track_counts off, so scans by those sessions are not counted", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, describeIndexUsage(tc.usage)) + }) + } +} diff --git a/pkg/preflight/index_usage.go b/pkg/preflight/index_usage.go new file mode 100644 index 00000000..ef23419a --- /dev/null +++ b/pkg/preflight/index_usage.go @@ -0,0 +1,175 @@ +package preflight + +import ( + "context" + "errors" + "fmt" + "strconv" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" +) + +// IndexUsage is the evidence of how an index has been used, read from the +// cumulative statistics of the one server the pool is connected to. Like +// TableUsage it describes that server alone: scans served by a streaming +// replica or an Aurora reader are counted there, not here. It is built only +// by ReadIndexUsage. +type IndexUsage struct { + // IdxScans is the cumulative count of scans of the index + // (pg_stat_all_indexes.idx_scan). For a partitioned index it is the sum + // over its leaf indexes, where the server records the scans; the + // partitioned index itself has no statistics. + IdxScans int64 + // LastIdxScan is when the server last recorded a scan of the index, or + // of any of a partitioned index's leaf indexes; nil when it recorded + // none since its statistics began, and always on PostgreSQL 14 and 15, + // which do not record scan times. + LastIdxScan *time.Time + // StatsReset is when the database's statistics were last reset; nil + // when the server reports none. An index created later has counted only + // since its creation, which the server does not record. + StatsReset *time.Time + // ReadAt is the server's now() for the read. + ReadAt time.Time + // Unique reports whether the index enforces uniqueness. + Unique bool + // Partitioned reports whether the index is a partitioned index, whose + // scans are counted on its leaf indexes. + Partitioned bool + // Partitions is the number of leaf indexes of a partitioned index, at + // any depth; 0 for an index that is not partitioned. + Partitions int + // TrackCounts is the reading session's track_counts; with it off the + // server counts no scans at all. + TrackCounts bool + // TrackCountsOffBySetting reports that a saved ALTER ROLE or ALTER + // DATABASE setting turns track_counts off for some role or for this + // database, so the scans of those sessions are not counted even when + // the reading session's TrackCounts is on. + TrackCountsOffBySetting bool +} + +// The index usage read: the scan counts of the index's leaf indexes (the +// index itself unless it is partitioned), its uniqueness and relkind, the +// database's statistics reset, the server's clock, and track_counts, from +// one statistics snapshot. indexUsageWithScanTimeQuery adds last_idx_scan, +// which PostgreSQL 16 introduced. The tree walks pg_inherits rather than +// calling pg_partition_tree, which locks every partition. Nothing here +// reads a relation itself, so the read takes no lock on the index or its +// table. statsRows counts the leaves the statistics view lists, so a +// missing row is told apart from a zero count. +// +// INV: CO-9 — every function, operator, relation, and type is +// pg_catalog-qualified. +const ( + indexUsageHead = ` + WITH RECURSIVE tree(relid) AS ( + SELECT $1::pg_catalog.oid + UNION ALL + SELECT h.inhrelid + FROM tree t + JOIN pg_catalog.pg_inherits h ON h.inhparent OPERATOR(pg_catalog.=) t.relid + JOIN pg_catalog.pg_class child ON child.oid OPERATOR(pg_catalog.=) h.inhrelid + WHERE child.relispartition + ), + leaves AS ( + SELECT s.indexrelid AS stats_row, s.idx_scan, ` + indexUsageTail = ` AS last_idx_scan + FROM tree t + JOIN pg_catalog.pg_class c ON c.oid OPERATOR(pg_catalog.=) t.relid + LEFT JOIN pg_catalog.pg_stat_all_indexes s ON s.indexrelid OPERATOR(pg_catalog.=) t.relid + WHERE c.relkind OPERATOR(pg_catalog.=) 'i' + ) + SELECT (SELECT pg_catalog.count(*) FROM leaves), + (SELECT pg_catalog.count(l.stats_row) FROM leaves l), + (SELECT COALESCE(pg_catalog.sum(l.idx_scan), 0)::pg_catalog.int8 FROM leaves l), + (SELECT pg_catalog.max(l.last_idx_scan) FROM leaves l), + (SELECT d.stats_reset FROM pg_catalog.pg_stat_database d + WHERE d.datname OPERATOR(pg_catalog.=) pg_catalog.current_database()), + pg_catalog.now(), i.indisunique, + ic.relkind OPERATOR(pg_catalog.=) 'I', + pg_catalog.current_setting('track_counts')::pg_catalog.bool + FROM pg_catalog.pg_index i + JOIN pg_catalog.pg_class ic ON ic.oid OPERATOR(pg_catalog.=) i.indexrelid + WHERE i.indexrelid OPERATOR(pg_catalog.=) $1::pg_catalog.oid` + indexUsageQuery = indexUsageHead + "NULL::pg_catalog.timestamptz" + indexUsageTail + indexUsageWithScanTimeQuery = indexUsageHead + "s.last_idx_scan" + indexUsageTail +) + +// errIndexUsageIndexNotPresent means the index the caller resolved was +// dropped before its usage was read. +var errIndexUsageIndexNotPresent = errors.New("the index no longer exists") + +// errIndexUsageStatisticsMissing means the statistics view lists no row for +// a leaf index, so its scans are unknown rather than zero. +var errIndexUsageStatisticsMissing = errors.New("the server reports no statistics for the index") + +// checkLeafStatisticsListed fails closed when the statistics view lists +// fewer rows than there are leaf indexes: an unlisted leaf's scans are +// unknown, and counting them as zero would make a used index look idle. +func checkLeafStatisticsListed(leaves, statsRows int) error { + if statsRows != leaves { + return fmt.Errorf("%w (%d of %d leaf indexes listed)", errIndexUsageStatisticsMissing, statsRows, leaves) + } + return nil +} + +// ReadIndexUsage reads the usage evidence of the index with the given OID +// on the server the pool is connected to, in one read-only transaction +// bounded by its own lock and statement timeouts, with every statistic from +// one snapshot. Keying on the OID reads the index the caller resolved, not +// whatever holds its name now. It changes nothing. +func ReadIndexUsage(ctx context.Context, pool *pgxpool.Pool, indexOID uint32) (IndexUsage, error) { + tx, err := pool.BeginTx(ctx, pgx.TxOptions{AccessMode: pgx.ReadOnly}) + if err != nil { + return IndexUsage{}, fmt.Errorf("begin index usage read %d: %w", indexOID, err) + } + defer func() { + // Redundant safety closer: after a successful Commit this returns + // the guaranteed ErrTxClosed. + _ = tx.Rollback(context.WithoutCancel(ctx)) + }() + // INV: LK-2 — the read bounds itself rather than trusting the pool's + // session defaults. + budgets := "SET LOCAL lock_timeout = " + strconv.FormatInt(usageReadLockTimeout.Milliseconds(), 10) + + "; SET LOCAL statement_timeout = " + strconv.FormatInt(usageReadStatementTimeout.Milliseconds(), 10) + if _, err := tx.Exec(ctx, budgets); err != nil { + return IndexUsage{}, fmt.Errorf("set index usage read budgets: %w", err) + } + if err := pinUsageStatsSnapshot(ctx, tx); err != nil { + return IndexUsage{}, err + } + var versionNum int + if err := tx.QueryRow(ctx, "SELECT pg_catalog.current_setting('server_version_num')::pg_catalog.int4").Scan(&versionNum); err != nil { + return IndexUsage{}, fmt.Errorf("read server version for the index usage read: %w", err) + } + q := indexUsageQuery + if versionNum >= scanTimesSinceVersionNum { + q = indexUsageWithScanTimeQuery + } + var u IndexUsage + var leaves, statsRows int + err = tx.QueryRow(ctx, q, indexOID).Scan(&leaves, &statsRows, &u.IdxScans, &u.LastIdxScan, + &u.StatsReset, &u.ReadAt, &u.Unique, &u.Partitioned, &u.TrackCounts) + if errors.Is(err, pgx.ErrNoRows) { + return IndexUsage{}, fmt.Errorf("read index usage %d: %w", indexOID, errIndexUsageIndexNotPresent) + } + if err != nil { + return IndexUsage{}, fmt.Errorf("read index usage %d: %w", indexOID, err) + } + if err := checkLeafStatisticsListed(leaves, statsRows); err != nil { + return IndexUsage{}, fmt.Errorf("read index usage %d: %w", indexOID, err) + } + if u.Partitioned { + u.Partitions = leaves + } + if u.TrackCountsOffBySetting, err = readUsageTrackCountsOffBySetting(ctx, tx); err != nil { + return IndexUsage{}, err + } + if err := tx.Commit(ctx); err != nil { + return IndexUsage{}, fmt.Errorf("commit index usage read %d: %w", indexOID, err) + } + return u, nil +} diff --git a/pkg/preflight/index_usage_integration_test.go b/pkg/preflight/index_usage_integration_test.go new file mode 100644 index 00000000..17c7a18a --- /dev/null +++ b/pkg/preflight/index_usage_integration_test.go @@ -0,0 +1,85 @@ +package preflight_test + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" + "github.com/block/pg-sprite/pkg/preflight" +) + +// indexOID resolves a fixture index to the OID ReadIndexUsage is keyed on. +func (f usageFixture) indexOID(t *testing.T, index string) uint32 { + t.Helper() + var oid uint32 + require.NoError(t, f.pool.QueryRow(t.Context(), + "SELECT to_regclass($1)::oid", f.schema+"."+index).Scan(&oid)) + return oid +} + +// ReadIndexUsage reports each index's own scan count, not the table's: of +// two indexes on one table, the one the test scans three times reads 3 and +// the unique one nothing scans reads 0. It reads the uniqueness of each, +// and, on PostgreSQL 16 and later, when the scanned one was last used. +func TestReadIndexUsageCountsEachIndexsOwnScans(t *testing.T) { + f := newUsageFixture(t, testutil.StartPostgres(t)) + f.exec(t, fmt.Sprintf(` + CREATE TABLE %[1]s.orders ( + id int PRIMARY KEY, + sku text NOT NULL, + created_at timestamptz NOT NULL + ); + INSERT INTO %[1]s.orders + SELECT g, 'sku-' || g, '2026-01-01'::timestamptz + g * interval '1 minute' + FROM generate_series(1, 1000) g; + CREATE INDEX orders_created_idx ON %[1]s.orders (created_at); + CREATE UNIQUE INDEX orders_sku_key ON %[1]s.orders (sku); + ANALYZE %[1]s.orders`, f.schema)) + + // Seq scans are disabled so each lookup scans orders_created_idx, the + // only index on created_at. + f.exec(t, "SET enable_seqscan = off") + for range 3 { + f.exec(t, fmt.Sprintf(` + SELECT id + FROM %s.orders + WHERE created_at = '2026-01-01 00:10:00+00'`, f.schema)) + } + f.exec(t, "RESET enable_seqscan") + f.publish(t) + + scanned := f.indexOID(t, "orders_created_idx") + var u preflight.IndexUsage + var readErr error + require.Eventually(t, func() bool { + if f.major < 15 { + if _, readErr = f.conn.Exec(t.Context(), "SELECT 1"); readErr != nil { + return true + } + } + u, readErr = preflight.ReadIndexUsage(t.Context(), f.pool, scanned) + return readErr != nil || u.IdxScans == 3 + }, statsVisible, 100*time.Millisecond, "the server must publish 3 scans of orders_created_idx") + require.NoError(t, readErr) + + assert.EqualValues(t, 3, u.IdxScans) + assert.False(t, u.Unique) + assert.True(t, u.TrackCounts) + assert.False(t, u.ReadAt.IsZero()) + if f.major >= 16 { + require.NotNil(t, u.LastIdxScan, "PostgreSQL 16 and later record when the index was last scanned") + assert.False(t, u.LastIdxScan.After(u.ReadAt), "the last scan cannot be later than the read") + } else { + assert.Nil(t, u.LastIdxScan, "PostgreSQL 14 and 15 record no scan times") + } + + unused, err := preflight.ReadIndexUsage(t.Context(), f.pool, f.indexOID(t, "orders_sku_key")) + require.NoError(t, err) + assert.EqualValues(t, 0, unused.IdxScans) + assert.True(t, unused.Unique) + assert.Nil(t, unused.LastIdxScan) +} diff --git a/pkg/preflight/index_usage_partitions_integration_test.go b/pkg/preflight/index_usage_partitions_integration_test.go new file mode 100644 index 00000000..1e49927b --- /dev/null +++ b/pkg/preflight/index_usage_partitions_integration_test.go @@ -0,0 +1,147 @@ +package preflight_test + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" + "github.com/block/pg-sprite/pkg/preflight" +) + +// A partitioned index has no statistics of its own: the server records its +// scans on the partitions' indexes. ReadIndexUsage sums them over every +// leaf index at any depth, here 3 lookups in one partition and 2 in a +// sub-partition's partition, and reports the partitioned index's leaves. +func TestReadIndexUsageSumsAPartitionedIndexsLeafIndexes(t *testing.T) { + f := newUsageFixture(t, testutil.StartPostgres(t)) + f.exec(t, fmt.Sprintf(` + CREATE TABLE %[1]s.events ( + id int NOT NULL, + ts timestamptz NOT NULL, + k int NOT NULL + ) PARTITION BY RANGE (ts); + CREATE TABLE %[1]s.events_2026 PARTITION OF %[1]s.events + FOR VALUES FROM ('2026-01-01') TO ('2027-01-01'); + CREATE TABLE %[1]s.events_2027 PARTITION OF %[1]s.events + FOR VALUES FROM ('2027-01-01') TO ('2028-01-01') + PARTITION BY RANGE (ts); + CREATE TABLE %[1]s.events_2027_h1 PARTITION OF %[1]s.events_2027 + FOR VALUES FROM ('2027-01-01') TO ('2027-07-01'); + CREATE INDEX events_k_idx ON %[1]s.events (k); + INSERT INTO %[1]s.events SELECT g, '2026-03-01', g FROM generate_series(1, 2000) g; + INSERT INTO %[1]s.events SELECT g, '2027-03-01', g FROM generate_series(1, 2000) g; + ANALYZE %[1]s.events`, f.schema)) + + // Seq scans are disabled so each lookup scans the partition's index. + f.exec(t, "SET enable_seqscan = off") + for range 3 { + f.exec(t, fmt.Sprintf(` + SELECT id + FROM %s.events + WHERE ts = '2026-03-01' AND k = 5`, f.schema)) + } + for range 2 { + f.exec(t, fmt.Sprintf(` + SELECT id + FROM %s.events + WHERE ts = '2027-03-01' AND k = 5`, f.schema)) + } + f.exec(t, "RESET enable_seqscan") + f.publish(t) + + parent := f.indexOID(t, "events_k_idx") + var u preflight.IndexUsage + var readErr error + require.Eventually(t, func() bool { + if f.major < 15 { + if _, readErr = f.conn.Exec(t.Context(), "SELECT 1"); readErr != nil { + return true + } + } + u, readErr = preflight.ReadIndexUsage(t.Context(), f.pool, parent) + return readErr != nil || u.IdxScans == 5 + }, statsVisible, 100*time.Millisecond, "the partitions' 5 scans must reach the partitioned index's usage") + require.NoError(t, readErr) + + assert.EqualValues(t, 5, u.IdxScans) + assert.True(t, u.Partitioned) + assert.Equal(t, 2, u.Partitions, "events_2026's index and events_2027_h1's; events_2027's own is partitioned, not a leaf") + if f.major >= 16 { + assert.NotNil(t, u.LastIdxScan, "the latest leaf scan time") + } + + leaf, err := preflight.ReadIndexUsage(t.Context(), f.pool, f.indexOID(t, "events_2026_k_idx")) + require.NoError(t, err) + assert.EqualValues(t, 3, leaf.IdxScans, "a leaf index reports its own scans") + assert.False(t, leaf.Partitioned) + assert.Zero(t, leaf.Partitions) +} + +// A partitioned index on a table with no partitions has nothing to scan: +// its count is a true zero over zero leaves, not a missing row read as 0. +func TestReadIndexUsageOfAPartitionedIndexWithNoPartitions(t *testing.T) { + f := newUsageFixture(t, testutil.StartPostgres(t)) + f.exec(t, fmt.Sprintf(` + CREATE TABLE %[1]s.events ( + ts timestamptz NOT NULL, + k int NOT NULL + ) PARTITION BY RANGE (ts); + CREATE INDEX events_k_idx ON %[1]s.events (k)`, f.schema)) + + u, err := preflight.ReadIndexUsage(t.Context(), f.pool, f.indexOID(t, "events_k_idx")) + require.NoError(t, err) + assert.True(t, u.Partitioned) + assert.Zero(t, u.Partitions) + assert.Zero(t, u.IdxScans) +} + +// An OID that names no index is refused, not reported as an unused index. +func TestReadIndexUsageRefusesAnOIDThatIsNotAnIndex(t *testing.T) { + f := newUsageFixture(t, testutil.StartPostgres(t)) + f.exec(t, fmt.Sprintf(` + CREATE TABLE %s.orders ( + id int PRIMARY KEY + )`, f.schema)) + + _, err := preflight.ReadIndexUsage(t.Context(), f.pool, f.indexOID(t, "orders")) + require.Error(t, err) +} + +// A role whose saved settings turn track_counts off scans an index without +// being counted, so the reading session's own track_counts does not prove +// the count complete; the read reports the saved setting. +func TestReadIndexUsageSeesTrackCountsTurnedOffForARole(t *testing.T) { + f := newUsageFixture(t, testutil.StartPostgres(t)) + f.exec(t, fmt.Sprintf(` + CREATE TABLE %[1]s.orders ( + id int PRIMARY KEY + )`, f.schema)) + var database string + require.NoError(t, f.conn.QueryRow(t.Context(), "SELECT current_database()").Scan(&database)) + // Roles are cluster-wide; the setting is scoped to this test's own + // database so no other test's read sees it. + role := pgx.Identifier{f.schema + "_untracked"}.Sanitize() + f.exec(t, "CREATE ROLE "+role) + t.Cleanup(func() { + _, err := f.pool.Exec(context.WithoutCancel(t.Context()), "DROP ROLE "+role) + assert.NoError(t, err) + }) + pkey := f.indexOID(t, "orders_pkey") + + f.exec(t, fmt.Sprintf(`ALTER ROLE %s IN DATABASE %s SET track_counts = on`, role, pgx.Identifier{database}.Sanitize())) + u, err := preflight.ReadIndexUsage(t.Context(), f.pool, pkey) + require.NoError(t, err) + assert.False(t, u.TrackCountsOffBySetting, "a saved setting that leaves track_counts on") + + f.exec(t, fmt.Sprintf(`ALTER ROLE %s IN DATABASE %s SET track_counts = off`, role, pgx.Identifier{database}.Sanitize())) + u, err = preflight.ReadIndexUsage(t.Context(), f.pool, pkey) + require.NoError(t, err) + assert.True(t, u.TrackCounts, "the reading session still counts") + assert.True(t, u.TrackCountsOffBySetting) +} diff --git a/pkg/preflight/index_usage_test.go b/pkg/preflight/index_usage_test.go new file mode 100644 index 00000000..0469ab2a --- /dev/null +++ b/pkg/preflight/index_usage_test.go @@ -0,0 +1,23 @@ +package preflight + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// A leaf index the statistics view does not list has unknown scans, so the +// read fails closed rather than counting it as zero. A live server lists +// every index of a table, which is why this is checked here and not through +// ReadIndexUsage. +func TestCheckLeafStatisticsListedRefusesAnUnlistedLeaf(t *testing.T) { + assert.ErrorIs(t, checkLeafStatisticsListed(2, 1), errIndexUsageStatisticsMissing) + assert.ErrorIs(t, checkLeafStatisticsListed(1, 0), errIndexUsageStatisticsMissing) +} + +// Every leaf listed, including a partitioned index with no partitions (zero +// leaves, zero rows), is a complete read. +func TestCheckLeafStatisticsListedAcceptsEveryLeafListed(t *testing.T) { + assert.NoError(t, checkLeafStatisticsListed(2, 2)) + assert.NoError(t, checkLeafStatisticsListed(0, 0)) +} diff --git a/pkg/preflight/table_usage.go b/pkg/preflight/table_usage.go index c372a6d7..28640fd2 100644 --- a/pkg/preflight/table_usage.go +++ b/pkg/preflight/table_usage.go @@ -254,9 +254,11 @@ func ReadTableUsage(ctx context.Context, pool *pgxpool.Pool, schema, table strin if err := readUsageServerFacts(ctx, tx, &u); err != nil { return TableUsage{}, err } - if err := readUsageTrackCountsSettings(ctx, tx, &u); err != nil { + offBySetting, err := readUsageTrackCountsOffBySetting(ctx, tx) + if err != nil { return TableUsage{}, err } + u.trackCountsOffBySetting = offBySetting tree, err := readUsageRelations(ctx, tx, &u) if err != nil { return TableUsage{}, err diff --git a/pkg/preflight/table_usage_coverage.go b/pkg/preflight/table_usage_coverage.go index ebb2329c..23429034 100644 --- a/pkg/preflight/table_usage_coverage.go +++ b/pkg/preflight/table_usage_coverage.go @@ -35,13 +35,13 @@ func pinUsageStatsSnapshot(ctx context.Context, tx pgx.Tx) error { return nil } -// readUsageTrackCountsSettings records on u whether a saved per-role or +// readUsageTrackCountsOffBySetting reports whether a saved per-role or // per-database setting turns track_counts off for some role or for this // database. The reading session's own value already reflects the server's // configuration and the reading role's settings; this catches the // settings of every other role. Each saved value was validated as a // boolean when it was set, so the cast cannot fail. -func readUsageTrackCountsSettings(ctx context.Context, tx pgx.Tx, u *TableUsage) error { +func readUsageTrackCountsOffBySetting(ctx context.Context, tx pgx.Tx) (bool, error) { // INV: CO-9 — every function, operator, relation, and type is // pg_catalog-qualified. const q = ` @@ -55,12 +55,11 @@ func readUsageTrackCountsSettings(ctx context.Context, tx pgx.Tx, u *TableUsage) WHERE pg_catalog.split_part(setting.entry, '=', 1) OPERATOR(pg_catalog.=) 'track_counts'` rows, err := tx.Query(ctx, q) if err != nil { - return fmt.Errorf("read track_counts settings: %w", err) + return false, fmt.Errorf("read track_counts settings: %w", err) } values, err := pgx.CollectRows(rows, pgx.RowTo[bool]) if err != nil { - return fmt.Errorf("read track_counts settings: %w", err) + return false, fmt.Errorf("read track_counts settings: %w", err) } - u.trackCountsOffBySetting = slices.Contains(values, false) - return nil + return slices.Contains(values, false), nil } diff --git a/pkg/verdict/verdict.go b/pkg/verdict/verdict.go index 4a07ec5b..7f4de39c 100644 --- a/pkg/verdict/verdict.go +++ b/pkg/verdict/verdict.go @@ -590,6 +590,64 @@ type Verdict struct { // StatisticsRefresh reports the ANALYZE a committed type change owes // the planner; nil when the statement retyped no column. StatisticsRefresh *StatisticsRefresh `json:"statistics_refresh,omitempty"` + // IndexUsage is the live usage evidence of the index a destructive-change + // refusal of an accepted DROP INDEX would have dropped; nil for every + // other verdict. + IndexUsage *IndexUsage `json:"index_usage,omitempty"` +} + +// IndexUsage is how the server pg-sprite is connected to has seen an index +// used, read when a DROP INDEX is refused for want of the destructive +// acknowledgement. It describes that one server only: every replica keeps +// its own counters, so an index this server never scans may still serve a +// replica's reads. +type IndexUsage struct { + // IdxScans is the cumulative count of scans of the index since + // StatsReset, or since the index was created if that is later; for a + // partitioned index, the sum over its leaf indexes. + IdxScans int64 `json:"idx_scan"` + // LastIdxScan is when the server last recorded a scan; absent when it + // recorded none, and always on PostgreSQL 14 and 15. In UTC. + LastIdxScan *time.Time `json:"last_idx_scan,omitempty"` + // StatsReset is when the database's statistics were last reset; absent + // when the server reports no reset. In UTC. + StatsReset *time.Time `json:"stats_reset,omitempty"` + // ReadAt is the server's clock when the evidence was read. In UTC. + ReadAt time.Time `json:"read_at"` + // Unique reports whether the index enforces uniqueness, a guarantee the + // drop would also discard. + Unique bool `json:"unique"` + // Partitions is the number of leaf indexes of a partitioned index, whose + // scans IdxScans sums; absent for an index that is not partitioned. + Partitions *int `json:"partitions,omitempty"` + // TrackCounts is false when the server was counting no scans, so a zero + // IdxScans proves nothing. + TrackCounts bool `json:"track_counts"` + // TrackCountsOffBySetting is true when a saved ALTER ROLE or ALTER + // DATABASE setting turns track_counts off, so the scans of those + // sessions are missing from IdxScans. + TrackCountsOffBySetting bool `json:"track_counts_off_by_setting"` +} + +// Summary renders the evidence on one line for the verdict's text form. +func (u IndexUsage) Summary() string { + s := fmt.Sprintf("%d index scans on this server (replicas not counted)", u.IdxScans) + if u.Partitions != nil { + s += fmt.Sprintf(" through its partition indexes (%d)", *u.Partitions) + } + if u.LastIdxScan != nil { + s += ", last " + u.LastIdxScan.UTC().Format(time.RFC3339) + } + if u.Unique { + s += ", unique" + } + if !u.TrackCounts { + s += ", track_counts off" + } + if u.TrackCountsOffBySetting { + s += ", track_counts off for some roles" + } + return s } // StatisticsRefresh is the outcome of re-analyzing a table after a @@ -666,6 +724,9 @@ func (v Verdict) String() string { if v.WritesBlocked { b.WriteString("\n blocked: writes to the table were refused (55006) while the run held it") } + if v.IndexUsage != nil { + fmt.Fprintf(&b, "\n usage: %s", v.IndexUsage.Summary()) + } if v.AllowedDestructive { b.WriteString("\n allowed: the statement's data loss was acknowledged (allow-destructive)") } diff --git a/pkg/verdict/verdict_test.go b/pkg/verdict/verdict_test.go index 0ad98e21..9ee8acd7 100644 --- a/pkg/verdict/verdict_test.go +++ b/pkg/verdict/verdict_test.go @@ -425,3 +425,44 @@ func TestStringIncludesAttemptsWhenSet(t *testing.T) { v.Attempts = 0 assert.NotContains(t, v.String(), "attempts") } + +// index_usage is part of the verdict contract an orchestrator reads: a zero +// scan count is present, not omitted, because 0 is the evidence the +// operator is asked to confirm, while the times the server did not record +// are absent rather than a zero time. +func TestIndexUsageJSONKeepsAZeroCountAndOmitsUnrecordedTimes(t *testing.T) { + readAt := time.Date(2026, 10, 28, 9, 30, 0, 0, time.UTC) + v := Verdict{ + Statement: "DROP INDEX app.orders_created_idx", + Table: "app.orders", + IndexUsage: &IndexUsage{ReadAt: readAt, Unique: true, TrackCounts: true}, + }.WithRefusal(ByDesign(ReasonDestructiveChange)) + + js, err := v.JSON() + require.NoError(t, err) + var got map[string]any + require.NoError(t, json.Unmarshal([]byte(js), &got)) + assert.Equal(t, map[string]any{ + "idx_scan": float64(0), + "read_at": "2026-10-28T09:30:00Z", + "unique": true, + "track_counts": true, + "track_counts_off_by_setting": false, + }, got["index_usage"]) + + lastScan := readAt.Add(-time.Hour) + partitions := 2 + v.IndexUsage.IdxScans = 42 + v.IndexUsage.LastIdxScan = &lastScan + v.IndexUsage.Partitions = &partitions + v.IndexUsage.TrackCounts = false + v.IndexUsage.TrackCountsOffBySetting = true + assert.Contains(t, v.String(), + "usage: 42 index scans on this server (replicas not counted) through its partition indexes (2), last 2026-10-28T08:30:00Z, unique, track_counts off, track_counts off for some roles") + + js, err = v.JSON() + require.NoError(t, err) + got = nil + require.NoError(t, json.Unmarshal([]byte(js), &got)) + assert.Equal(t, float64(2), got["index_usage"].(map[string]any)["partitions"], "a partitioned index's leaf count") +}