Merge pull request #1692 from dcoric/feat/postgres-pushes-perf

perf: index the PostgreSQL pushes hot paths and slim list projections

Author: dcoric

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<PushQuery> = defaultPushQuery): Promise<Action[]> => {
   const clauses: string[] = [];
   const values: unknown[] = [];
@@ -181,7 +184,7 @@ export const getPushes = async (q: Partial<PushQuery> = 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);

src/db/postgres/schemaMigrations.ts

@@ -145,6 +145,31 @@ export const MIGRATIONS: Migration[] = [
     // legacy JSONB column (backfilled in migration 4) is no longer used.
     sql: `ALTER TABLE repos DROP COLUMN IF EXISTS users;`,
   },
+  {
+    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';
+`,
+  },
 ];
 
 const SCHEMA_MIGRATIONS_TABLE_SQL = `

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: [] });

test/db/postgres/schemaMigrations.integration.test.ts

@@ -53,7 +53,7 @@ describe.runIf(shouldRunPostgresTests)('PostgreSQL Schema Migration Integration
     const versions = await query<{ version: number }>(
       'SELECT version FROM schema_migrations ORDER BY version',
     );
-    expect(versions.rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5]);
+    expect(versions.rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5, 6]);
 
     const tables = await query<{ tablename: string }>(
       `SELECT tablename FROM pg_tables
@@ -90,7 +90,7 @@ describe.runIf(shouldRunPostgresTests)('PostgreSQL Schema Migration Integration
     await connect();
 
     const versions = await query<{ version: number }>('SELECT version FROM schema_migrations');
-    expect(versions.rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5]);
+    expect(versions.rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5, 6]);
   });
 
   it('backfills existing JSONB repo permissions into repo_users (single, multi, same user in both roles)', async () => {
@@ -143,7 +143,7 @@ describe.runIf(shouldRunPostgresTests)('PostgreSQL Schema Migration Integration
       const versions = await pool.query<{ version: number }>(
         'SELECT version FROM schema_migrations ORDER BY version',
       );
-      expect(versions.rows.map((r) => r.version)).toEqual([1, 2, 3, 4, 5]);
+      expect(versions.rows.map((r) => r.version)).toEqual([1, 2, 3, 4, 5, 6]);
 
       // The legacy JSONB column is gone, dropped by v4.
       const usersCol = await pool.query(

test/db/postgres/schemaMigrations.test.ts

@@ -38,6 +38,18 @@ const makePool = (appliedRows: { version: number }[] = []) => {
 const sqlsOf = (query: ReturnType<typeof vi.fn>) => 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);