From a0abca02249ce33ec8dbe963dca982d10aa9ea82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Mon, 24 Aug 2026 12:18:43 +0200 Subject: [PATCH] perf(db/postgres): index the pushes hot paths and slim list projections Push rows carry the full diff inside steps, so queries that read the data JSONB detoast very large rows. Three hot paths paid for that: - the repo activity rollup scanned and detoasted every push row on every repos page; a covering expression index makes it an index-only scan that never touches the heap - the default dashboard query gets a matching partial index, and the user profile predicates get expression indexes - list queries now select data minus the steps key, matching the mongo backend's list projection, which also excludes steps; the push detail view still returns the full document Indexes ship as schema migration v6. Versions 4 and 5 are reserved by the repo_users normalisation branch, and the runner applies in version order, so the gap is harmless. --- src/db/postgres/pushes.ts | 7 ++++-- src/db/postgres/schemaMigrations.ts | 28 +++++++++++++++++++++++ test/db/postgres/pushes.test.ts | 17 ++++++++++++++ test/db/postgres/schemaMigrations.test.ts | 12 ++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/db/postgres/pushes.ts b/src/db/postgres/pushes.ts index 48481b3a8..ba1417425 100644 --- a/src/db/postgres/pushes.ts +++ b/src/db/postgres/pushes.ts @@ -163,12 +163,15 @@ export const getPushesForUserProfile = async ( } const result = await query<{ data: unknown }>( - `SELECT data FROM pushes WHERE type = 'push' AND ${predicate} ORDER BY timestamp DESC`, + `SELECT data - 'steps' AS data FROM pushes WHERE type = 'push' AND ${predicate} ORDER BY timestamp DESC`, values, ); return result.rows.map(rowToAction); }; +// List queries drop `steps` from the returned document: it holds the full diff +// (largest part of a push row) and the mongo backend's list projection excludes +// it as well. The push-detail path (`getPush`) still returns the whole document. export const getPushes = async (q: Partial = defaultPushQuery): Promise => { const clauses: string[] = []; const values: unknown[] = []; @@ -181,7 +184,7 @@ export const getPushes = async (q: Partial = defaultPushQuery): Promi const where = clauses.length > 0 ? `WHERE ${clauses.join(' AND ')}` : ''; const result = await query<{ data: unknown }>( - `SELECT data FROM pushes ${where} ORDER BY timestamp DESC`, + `SELECT data - 'steps' AS data FROM pushes ${where} ORDER BY timestamp DESC`, values, ); return result.rows.map(rowToAction); diff --git a/src/db/postgres/schemaMigrations.ts b/src/db/postgres/schemaMigrations.ts index c92a8385e..87890ad3e 100644 --- a/src/db/postgres/schemaMigrations.ts +++ b/src/db/postgres/schemaMigrations.ts @@ -105,6 +105,34 @@ export const MIGRATIONS: Migration[] = [ CREATE TABLE IF NOT EXISTS migrations ( id TEXT PRIMARY KEY ); +`, + }, + // Versions 4 and 5 are reserved by the repo_users normalisation branch + // (repo_users_table and drop_repos_users_jsonb). The runner applies by + // version order, so the gap is harmless until those entries land. + { + version: 6, + name: 'pushes_hot_path_indexes', + sql: ` + -- Covering index for the repo activity rollup: the scan becomes index-only + -- and never detoasts the large push JSONB documents. + CREATE INDEX IF NOT EXISTS pushes_rollup_idx + ON pushes ((data->>'url'), timestamp) + INCLUDE (error, rejected, canceled, authorised, blocked, allow_push) + WHERE type = 'push'; + + -- Matches the default dashboard query for pushes pending review. + CREATE INDEX IF NOT EXISTS pushes_pending_idx + ON pushes (timestamp DESC) + WHERE type = 'push' AND blocked AND NOT error AND NOT authorised AND NOT allow_push; + + -- User profile lookups filter on JSONB expressions; index both predicates. + CREATE INDEX IF NOT EXISTS pushes_user_email_idx + ON pushes ((data->>'userEmail')) + WHERE type = 'push'; + CREATE INDEX IF NOT EXISTS pushes_reviewer_idx + ON pushes ((lower(data->'attestation'->'reviewer'->>'username'))) + WHERE type = 'push'; `, }, ]; diff --git a/test/db/postgres/pushes.test.ts b/test/db/postgres/pushes.test.ts index 1bcadd3bc..8e92f7847 100644 --- a/test/db/postgres/pushes.test.ts +++ b/test/db/postgres/pushes.test.ts @@ -213,6 +213,23 @@ describe('PostgreSQL - Pushes', async () => { }); }); + describe('list projection', () => { + it('drops steps from list results but not from the detail view', async () => { + mockQuery.mockResolvedValue({ rowCount: 0, rows: [] }); + + await getPushes({}); + await getPushesForUserProfile([], 'alice'); + await getPush('p1'); + + const [listSql] = mockQuery.mock.calls[0]; + const [profileSql] = mockQuery.mock.calls[1]; + const [detailSql] = mockQuery.mock.calls[2]; + expect(listSql).toContain("data - 'steps'"); + expect(profileSql).toContain("data - 'steps'"); + expect(detailSql).not.toContain("data - 'steps'"); + }); + }); + describe('getPushesForUserProfile', () => { it('matches the reviewer case-insensitively when there are no emails', async () => { mockQuery.mockResolvedValue({ rowCount: 0, rows: [] }); diff --git a/test/db/postgres/schemaMigrations.test.ts b/test/db/postgres/schemaMigrations.test.ts index b1f933675..d0a74e659 100644 --- a/test/db/postgres/schemaMigrations.test.ts +++ b/test/db/postgres/schemaMigrations.test.ts @@ -38,6 +38,18 @@ const makePool = (appliedRows: { version: number }[] = []) => { const sqlsOf = (query: ReturnType) => query.mock.calls.map((call) => String(call[0])); describe('PostgreSQL - migrations', () => { + it('defines the pushes hot-path indexes as version 6', () => { + const v6 = MIGRATIONS.find((m) => m.version === 6); + expect(v6?.name).toBe('pushes_hot_path_indexes'); + expect(v6?.sql).toContain('pushes_rollup_idx'); + expect(v6?.sql).toContain( + 'INCLUDE (error, rejected, canceled, authorised, blocked, allow_push)', + ); + expect(v6?.sql).toContain('pushes_pending_idx'); + expect(v6?.sql).toContain('pushes_user_email_idx'); + expect(v6?.sql).toContain('pushes_reviewer_idx'); + }); + it('exposes an ordered, append-only migration list starting at version 1', () => { expect(MIGRATIONS[0].version).toBe(1);