fix: gauntlet cycle 3 must-fixes for PR-5.1.5 (operator-runbook drift)

Cycle 3 surfaced operator-facing doc drift from this PR adding the
requires-superuser marker to 006/007 (the same class as cycle 2's
migrations/README.md fix, in the sibling infra runbooks):

- benchmarks-website/infra/README.md bootstrap section no longer claims only
  001/002/003 are master-applied with later additive migrations run as migrator;
  it now names the in-file marker as authoritative and points to
  migrations/README.md's bootstrap-ordering contract (002/004/005 + 006/007).
- benchmarks-website/infra/provision.sh's operator handoff no longer lists only
  "002 + 004" as master-applied; it instructs applying every marked migration
  as master and references the migration docs so the list cannot drift again.

Plus the cycle-3 should-fixes:
- test_real_bootstrap_migrations_carry_superuser_marker now pins the marker on
  006/007 too, so a future marker removal fails the suite.
- migrations/README.md re-stamp note states the real privilege requirement
  (UPDATE on query_measurements AND SELECT on commits).
- The commit_timestamp write-path test asserts over ALL rows (count of
  unstamped/drifted == 0) instead of an arbitrary fetchone row.
- Aligned a misindented UNION arm in the filter-universe formats query.

Deferred (nit): trimming the summary.ts skip-scan comment block (it documents
load-bearing non-obvious planner behavior, not a hack).

This is the final inner-loop cycle (project ~3-cycle cap); all cycle-3 findings
are resolved. The fixes touch operator docs + tests only, no production logic.

Signed-off-by: "Connor Tsui" <connor@spiraldb.com>

Author: connortsui20

benchmarks-website/infra/README.md

@@ -156,21 +156,26 @@ export PGSSLROOTCERT=/tmp/rds-global-bundle.pem
 uv run --no-project scripts/migrate-schema.py apply
 ```
 
-This applies `001` (schema), `002` (creates the `migrator` role + binds it to
-`rds_iam`), and `003` (grants `migrator` SELECT + INSERT on the
-`_applied_migrations` ledger). After the bootstrap, subsequent *additive*
+This applies `001` (schema) and every migration carrying the
+`-- migrate-schema: requires-superuser` marker — currently `002`/`004`/`005`
+(role creation + grants) and `006`/`007` (DDL on the master-owned
+`query_measurements` table). The marker in each file is authoritative; the
+runner refuses to apply a marked file under a non-master role. The single source
+of truth for the bootstrap-ordering contract is
+[`migrations/README.md`](../../migrations/README.md) § "Bootstrap ordering —
+`requires-superuser` migrations"; apply every marked migration as the master
+here. `003` (a ledger grant) carries no marker. After the bootstrap, *unmarked*
 migrations are applied by the `schema-deploy` workflow as `migrator` against the
 public instance endpoint (`RDS_BENCH_INSTANCE_ENDPOINT`) with direct IAM; the
-master password is not needed again.
+master password is not needed again for those.
 
 Because the bootstrap runs as master, the schema objects and the ledger are
 master-owned; `003` grants `migrator` the ledger access it needs to record and
 read applied migrations. A migration that `ALTER`s or adds an index to an
-existing master-owned table (rather than only `CREATE`-ing new objects) needs
-the role-ownership model resolved first -- deferred to PR-2.1 alongside the
-ingest-role design (which also grants the six data tables' DML for the ingest
-write path). Phase-1 migrations are all additive (new objects), so `migrator`'s
-`CREATE` on the schema suffices today.
+existing master-owned table (rather than only `CREATE`-ing new objects) must
+itself carry the `requires-superuser` marker and be master-applied here — this
+is what `006`/`007` do (PR-5.1.5). `migrator`'s `CREATE` on the schema suffices
+only for new-object migrations.
 
 The per-PR testcontainer migration test (`scripts/test_migrate_schema.py`)
 applies every migration as a single owning role, so it does NOT model the

benchmarks-website/infra/provision.sh

@@ -611,9 +611,13 @@ NEXT STEPS:
 
 4. Migrations create the Postgres '${PG_MIGRATOR_ROLE}' (002) and
    '${PG_INGEST_ROLE}' (004) users; both OIDC roles' permission policies are
-   already scoped to those users. Apply 002 + 004 as the RDS master during the
-   one-time bootstrap (they create roles and grant on master-owned tables), after
-   which schema deploys run as '${PG_MIGRATOR_ROLE}'.
+   already scoped to those users. During the one-time bootstrap, apply EVERY
+   migration carrying the '-- migrate-schema: requires-superuser' marker as the
+   RDS master (currently 002/004/005, which create roles + grants, and 006/007,
+   which run DDL on the master-owned query_measurements table). The marker in
+   each file is authoritative; see benchmarks-website/migrations/README.md
+   'Bootstrap ordering' for the contract. After the bootstrap, unmarked schema
+   deploys run as '${PG_MIGRATOR_ROLE}'.
 
 =========================================================================
 EOF

benchmarks-website/web/lib/queries.ts

@@ -1231,7 +1231,7 @@ export async function collectFilterUniverse(): Promise<FilterUniverse> {
              LIMIT 1
           ) nxt
       )
-            SELECT value FROM qm_formats
+      SELECT value FROM qm_formats
       UNION SELECT format AS value FROM compression_times    WHERE format IS NOT NULL
       UNION SELECT format AS value FROM compression_sizes    WHERE format IS NOT NULL
       UNION SELECT format AS value FROM random_access_times  WHERE format IS NOT NULL`,

migrations/README.md

@@ -77,8 +77,10 @@ read-service identity), and `006`/`007` in PR-5.1.5 (read-path perf).
 `commits.timestamp` (006). Writers stamp it at insert; 006 backfilled
 pre-existing rows. If rows are ever observed unstamped or drifted (a writer
 predating the stamp, or a commits re-ingest that changed a timestamp), the
-repair is the drift-tolerant form of the backfill, run as any role with UPDATE
-on the table:
+repair is the drift-tolerant form of the backfill. Run it as a role that holds
+`UPDATE` on `query_measurements` **and** `SELECT` on `commits` (the `FROM
+commits` clause reads `commits.timestamp`) — the RDS master, or any role granted
+both; the least-privilege `bench_read` role cannot run it:
 
 ```sql
 UPDATE query_measurements q

scripts/test_migrate_schema.py

@@ -1712,15 +1712,22 @@ def test_migration_requires_superuser_false_without_exact_marker() -> None:
 
 
 def test_real_bootstrap_migrations_carry_superuser_marker() -> None:
-    """002 + 004 + 005 (role/grant bootstrap) are marked; 001 + 003 (additive / owned-grant) are not."""
+    """002/004/005 (role/grant bootstrap) + 006/007 (master-owned-table DDL) are marked;
+    001 + 003 (additive / owned-grant) are not."""
     by_name = {
         p.name: p.read_text(encoding="utf-8") for p in runner.discover(REPO_MIGRATIONS_DIR)
     }
-    for marked in ("002_iam_db_user.sql", "004_ingest_role.sql", "005_read_role.sql"):
+    for marked in (
+        "002_iam_db_user.sql",
+        "004_ingest_role.sql",
+        "005_read_role.sql",
+        "006_read_path_perf.sql",
+        "007_summary_covering_index.sql",
+    ):
         assert marked in by_name, f"expected {marked} in migrations/"
         assert runner._migration_requires_superuser(by_name[marked]), (
-            f"{marked} creates roles / runs ALTER DEFAULT PRIVILEGES and must carry the "
-            "requires-superuser marker"
+            f"{marked} creates roles / runs ALTER DEFAULT PRIVILEGES / ALTERs a master-owned "
+            "table and must carry the requires-superuser marker"
         )
     for unmarked in ("001_initial_schema.sql", "003_migrator_ledger_grant.sql"):
         assert unmarked in by_name, f"expected {unmarked} in migrations/"

scripts/test_post_ingest_postgres.py

@@ -309,21 +309,25 @@ def test_query_measurement_insert_populates_commit_timestamp(schema_conn: psycop
     writer never depend on the post-deploy re-backfill."""
     commit = _sample_commit()
     records = [r for r in _sample_records() if r["kind"] == "query_measurement"]
+    assert records, "expected at least one query_measurement sample record"
+
+    def _unstamped_or_drifted() -> int:
+        # Count rows that are NOT stamped to their commit's timestamp -- 0 means every
+        # query_measurements row was correctly stamped (not just an arbitrary fetchone row).
+        return schema_conn.execute(
+            "SELECT count(*) FROM query_measurements q JOIN commits c USING (commit_sha)"
+            " WHERE q.commit_timestamp IS NULL OR q.commit_timestamp <> c.timestamp"
+        ).fetchone()[0]
 
     post_ingest.ingest_postgres(schema_conn, commit, records)
-    row = schema_conn.execute(
-        "SELECT q.commit_timestamp, c.timestamp FROM query_measurements q"
-        " JOIN commits c USING (commit_sha)"
-    ).fetchone()
-    assert row[0] is not None
-    assert row[0] == row[1]
+    assert _count(schema_conn, "query_measurements") == len(records)
+    assert _unstamped_or_drifted() == 0
 
     # The update path re-stamps as well: scrub the column, re-ingest the same envelope (an
-    # ON CONFLICT DO UPDATE), and the timestamp must come back.
+    # ON CONFLICT DO UPDATE), and every row's timestamp must come back.
     schema_conn.execute("UPDATE query_measurements SET commit_timestamp = NULL")
     post_ingest.ingest_postgres(schema_conn, commit, records)
-    restamped = schema_conn.execute("SELECT commit_timestamp FROM query_measurements").fetchone()[0]
-    assert restamped == row[1]
+    assert _unstamped_or_drifted() == 0
 
 
 # Per-kind mutation of every value/side-counter/env column each table's ON