Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/db/postgres/pushes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand All @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions src/db/postgres/schemaMigrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `
Expand Down
17 changes: 17 additions & 0 deletions test/db/postgres/pushes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [] });
Expand Down
6 changes: 3 additions & 3 deletions test/db/postgres/schemaMigrations.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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(
Expand Down
12 changes: 12 additions & 0 deletions test/db/postgres/schemaMigrations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading