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 drops npm_versions, so npm_packages and maven_packages stand 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's versions_count recount 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, leaving npm_packages and maven_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 an Index Cond leading with the join term. PostgreSQL 18's skip scan picks the new package-id index and places the namespace_id equality 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:dupl token on the recovery walk is load-bearing: removing it flags npm_versions_package_id_index_schema_integration_test.go:322-395 against the container twin at :863-943.
  • structure.sql regenerated via mise run db:dump-structure: the parent, all 64 children, and all 64 attaches are present. migrations_checksum_test.go moves knownHeadVersion to 20260918120000.
  • 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 parent DELETE prints 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 the namespace_id equality first.
  • The db:migrate up/down/re-up matrix and test:integration:migrations are 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 Files list, 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 on main today (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)

Edited by Moaz Khalifa

Merge request reports

Loading
Loading