fix(datastore): serve the npm_versions package-id FK check via a non-partial index (package-id-fk-check-indexes plan: 2/2)
What this does
Step 2 of the package-id FK-check indexes plan (work item #684 (closed); the plan is the governing record, and no spec section owns the FK-check index shape).
Adds index_npm_versions_on_ns_id_pkg_id_id on npm_versions (namespace_id, npm_package_id, id), non-partial, over the 64 hash partitions by the container-twin path: ON ONLY parent, one DROP INDEX CONCURRENTLY IF EXISTS plus one CREATE INDEX CONCURRENTLY IF NOT EXISTS per partition, an attach phase under lock_timeout = '5s', and a Down that drops the parent and cascades.
The leading pair is a prefix the fk_npm_versions_npm_package_id_npm_packages check can seek. Every npm_versions index leading (namespace_id, npm_package_id) is partial on soft_deleted_at IS NULL, and both non-partial indexes lead namespace_id without carrying the package column, so the check reads the namespace's rows in the partition. Note 3703947624 measures the unserved check at 72 buffers, where the two served checks on the same npm_packages delete read 2 each.
id is the third column because the repository walk's nested loop probes (namespace_id, npm_package_id) as the inner index for npmVersionsReapPageStmt, and that statement projects npm_versions.id and nothing else, so the inner scan turns index-only. The walk consumer is unmeasured for npm; recording id is the plan's answer to the column-order question note 3703947624 leaves open.
No existing npm_versions index carries (namespace_id, npm_package_id, id), so the per-partition children are spelled byte-identical to the auto-generated npm_versions_pNN_namespace_id_npm_package_id_id_idx, and the parent needs no _all suffix.
What else moves
The claims this index falsifies:
docs/dev/storage-accounting.md: the probe paragraph's remaining sentence dropsnpm_versions, sonpm_packagesandmaven_packagesstand as the two tables with no drivable index, and the split paragraph collapses to one plan.docs/specs/S11-npm-hosted.md: the publish section'sversions_countrecount claim — every index leading(namespace_id, npm_package_id)partial, the recount falling back to a namespace-wide scan — is rewritten to the index-servable, not cap-bounded seek this index provides.lifecycle_reap_repository_explain_integration_test.go: the header's no-scan-shape list drops both version tables, leavingnpm_packagesandmaven_packages. The 64-line header was an over-cap block that this edit touches, so the comment caps compress it to its two-line load-bearing claim.internal/datastore/npm_virtual_hosted_lookup_integration_test.go(found by CI): the tarball lookup's probe pin accepted only anIndex Condleading with the join term. PostgreSQL 18's skip scan picks the new package-id index and places thenamespace_idequality first, so the assertion now accepts either conjunct order — the direction claim it guards is unchanged.
Evidence
Local gates:
lint:migrations(squawk 2.64.0) on the new migration: 0 issues. The two statements that fire carry statement-level suppressions with the reason on the same line.golangci-lint run --build-tags=integration(2.13.2) over the migrations package: no findings on the new file. The//nolint:dupltoken on the recovery walk is load-bearing: removing it flagsnpm_versions_package_id_index_schema_integration_test.go:322-395against the container twin at:863-943.structure.sqlregenerated viamise run db:dump-structure: the parent, all 64 children, and all 64 attaches are present.migrations_checksum_test.gomovesknownHeadVersionto20260918120000.- The new suite covers the parent shape, the exact key tuple, the empty predicate, all 64 children with their spelled names, the identifier-length fit, the Down and the staged rollback, the crash re-run, and the prune-to-one-partition EXPLAIN. The prune case runs the referential-integrity query itself under
EXPLAIN (ANALYZE, BUFFERS)and asserts that the plan names a catalog-resolved partition child of the new index — the parentDELETEprints no trigger plan by design. - The lookup pin passes on PostgreSQL 16.14, 17.11, and 18.6 locally (one test per version against a local server through
ARTIFACT_REGISTRY_DATABASE_TEST_DSN): 16 and 17 probe through the primary key with the join term first, 18 through the new index with thenamespace_idequality first. - The
db:migrateup/down/re-up matrix andtest:integration:migrationsare CI-owned.
ADR conformance
ADR 007 governs the schema. The amended repository-level reconciliation clause (mirror: docs/adr/007_database_schema.md:917, :2220) records the maven_files index Step 1 landed; this step adds a plain non-partial btree over the existing HASH(namespace_id)/64-partition layout and introduces no insurance structure, shadow table, or reconciliation mechanism. The branch touches no file under docs/adr/. The plan's Step 1 handbook gate (gitlab-com/content-sites/handbook!21168 (merged)) merged and is mirrored on main before this branch's fork point.
Notes
- No e2e scenario is added or changed: no protocol surface, no response, no route. Conformance runs do not apply for the same reason.
- No user-facing documentation is affected: the change alters which physical plan a database-level delete takes.
- Diff size: 8 files, +1358/−83.
structure.sql(+258) is generated; the migration (+564) is the per-partition build/attach pattern repeated for 64 partitions, mirroring the in-tree container twin; the new suite (+520) carries the shape, Down, re-run, and pruning coverage; the remaining lines are the falsified-claim updates, including the ~85-line net comment trim. Splitting would separate the index from the claims it falsifies, and the plan's two steps already split the Maven pair from the npm single for the serialization the shared files need. - Three paths ride outside Step 2's plan
Fileslist, all falsified-claim upkeep: the explain-suite header (the plan assigned its npm edit to Step 1's entry, but Step 1 merged without performing it), the S11 rectification (the plan declared Spec N/A, but the index falsifies a recount-cost fact the plan had not catalogued), and the npm virtual hosted lookup pin (CI found it: PostgreSQL 18's skip scan changes the probe's conjunct order). Step MRs cannot edit the plan file, so the reconciliation lives here. - The comment compression is form-only: the test-file diff between the implementation commit and its review-advisory commit changes no statement, assertion, or signature, and the explain header's collapse is what the comment caps force on a legacy block an edit touches. The code-simplifier pass then restored the antecedent one compressed comment had lost (
npm_versions_package_id_index_schema_integration_test.go:488). - Tagged
golangci-lint(--build-tags=integration) reports corpus-wide pre-existing findings onmaintoday (wsl,errcheck,contextcheck, and others); the files in this MR contribute none to the union of changed files.
Database Review Evidence
Migrations
Note
Timings are from CI (db:migrate matrix, goose verbose) against an
empty database, in apply / rollback order per PG version.
Production-scale validation via Database Lab is not yet available. See
Database review evidence
for the matrix rationale and how to read the numbers.
| Migration | PG 16 | PG 17 | PG 18 |
|---|---|---|---|
20260918120000_add_npm_versions_package_id_index.sql |
OK (621.32ms / 21.31ms) | OK (257.94ms / 15.28ms) | OK (310.71ms / 21.06ms) |
Queries
Query mode does not apply: every changed Go file is a test file, so no query-producing method changed.
Related to #684 (closed)