Skip to content

perf(db): index the foreign keys and list sort columns - #317

Merged
nourshoreibah merged 2 commits into
mainfrom
worktree-db-indexes
Aug 12, 2026
Merged

perf(db): index the foreign keys and list sort columns#317
nourshoreibah merged 2 commits into
mainfrom
worktree-db-indexes

Conversation

@nourshoreibah

Copy link
Copy Markdown
Collaborator

Why

The schema had eleven indexes, and every one of them was a primary key or a UNIQUE constraint. Not one covered a foreign key — Postgres doesn't create those for you.

That means every lookup by project_id, every date-ordered list page, and every ON DELETE CASCADE check was a sequential scan of the entire child table. This migration adds the seven indexes the code actually asks for.

How the access patterns were determined

Full inventory of every query in apps/backend/lambdas/* and shared/lambda-auth/, plus the two open PRs (#310, #315) so this doesn't get invalidated the moment they merge.

Findings that shaped the index set:

  • project_memberships and project_donations both have a UNIQUE constraint whose leading column is the wrong one for how they're queried. UNIQUE is (project_id, user_id) but GET /projects for a non-admin looks up by user_id alone — and that's the landing page of the app. Same story for project_donations: UNIQUE is (donor_id, project_id), but everything filters on project_id.
  • The authz lookups on project_memberships filter on both project_id and user_id, so the existing UNIQUE already serves them. No index needed there.
  • authenticate.ts:55 (users WHERE cognito_sub = $1) runs on every authenticated request, but cognito_sub is already UNIQUE. Nothing to do.
  • The expenditure list queries all pair a project_id filter with ORDER BY spent_on DESC, hence the composite rather than a bare FK index — the second column removes the sort node entirely.

Measurements

Local Postgres 16, schema built from the migrations in this repo, loaded with 500k expenditures / 50k memberships / 50k donations / 20k reports / 5k users / 2k projects. EXPLAIN (ANALYZE, BUFFERS), warm cache. Buffer counts are the honest metric here — wall-clock on a warm local container flatters everything.

Query Before After
Project-filtered expenditure page 6846 buffers, 48.4 ms 28 buffers, 0.8 ms
Unfiltered expenditure page 6846 buffers, 23.8 ms 28 buffers, 1.2 ms
GET /projects for a non-admin 406 buffers, 6.2 ms 34 buffers, 1.1 ms
Project-filtered report page 230 buffers, 7.8 ms 15 buffers, 2.3 ms
GET /projects/{id}/donors 423 buffers, 7.4 ms 82 buffers
DELETE one project (FK cascade) 179.2 ms 42.0 ms
DELETE one user (FK cascade) 127.4 ms 38.8 ms

The expenditure list went from reading ~53 MB per request to ~220 KB.

For the cascades, the time was almost entirely in one FK trigger: expenditures_project_id_fkey alone was 146 ms of the 179 ms project delete, and expenditures_entered_by_fkey was 108 ms of the 127 ms user delete.

Cost

Seven indexes, ~11 MB total at the row counts above. expenditures carries three of them, so its write path takes three extra index maintenance ops per insert — acceptable for a table that is read on nearly every page of the app and appended to a few times a day.

Notes on what is deliberately not here

  • No index on expenditures.status. The new admin review queue in feat(expenses): admin approve/deny review flow with receipt upload #315 filters by status client-side — it fetches the list and narrows to pending in the browser. There is no SQL predicate on status, so an index (even a partial one) would never be used. If that filter moves server-side, CREATE INDEX ... ON expenditures (status, spent_on) WHERE status = 'pending' becomes worth adding; pending is a ~15% minority of rows, so a partial index would be well-targeted.
  • No index on expenditures.category. The dashboard's two aggregates (projects/handler.ts:75 and :83) are inherently full scans — one is a GROUP BY project_id over the whole table, and the other's category IS NOT NULL predicate matches ~100% of rows. Measured 86 ms and 51 ms, and no index changes that. projects/handler.ts:83 streams every expenditure row into the Lambda and buckets it by month in a JS loop; that endpoint needs a server-side aggregate, not an index. Worth a follow-up issue.
  • Indexes are declared ASC even where queries sort DESC. Both sort columns are NOT NULL, so Postgres reads the index backwards (Index Scan Backward) and gets the ordering for free — confirmed in the plans above. A DESC index would buy nothing.

Safety

Adding an index is pure expand: no column changes, no live INSERT breaks, and the currently deployed code only gets faster. Satisfies the "safe for the code that is live right now" rule in apps/backend/db/README.md.

Plain CREATE INDEX, not CONCURRENTLY — the migrator wraps the run in a single transaction, and CI rejects CONCURRENTLY for that reason.

Verification

  • Applied all four migrations to an empty database in order — clean, as migrations-fresh does it.
  • seed.sql applies on top of the migrated schema.
  • Confirmed 11 → 18 indexes afterwards.
  • Ran the migrations-guard filename regex and the destructive-SQL/CONCURRENTLY scan locally; both pass.
  • No shared/types/db-types.d.ts change — indexes don't alter generated column types.

🤖 Generated with Claude Code

Every index in the schema was a primary key or a UNIQUE constraint. None
covered a foreign key, so lookups by project_id, date-ordered list pages,
and ON DELETE CASCADE checks were all sequential scans of the child table.

Measured on a local Postgres 16 loaded with 500k expenditures, 50k
memberships, 50k donations, 20k reports, 5k users, 2k projects:

  project-filtered expenditure page   6846 buffers, 48.4ms -> 28 buffers, 0.8ms
  unfiltered expenditure page         6846 buffers, 23.8ms -> 28 buffers, 1.2ms
  GET /projects for a non-admin        406 buffers,  6.2ms -> 34 buffers, 1.1ms
  project-filtered report page         230 buffers,  7.8ms -> 15 buffers, 2.3ms
  GET /projects/{id}/donors            423 buffers,  7.4ms -> 82 buffers
  DELETE one project (FK cascade)               179.2ms -> 42.0ms
  DELETE one user (FK cascade)                  127.4ms -> 38.8ms

Seven indexes, ~11MB total at that row count. All additive, so they are
safe for the currently deployed code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

This PR contains a database migration

  • apps/backend/db/migrations/20260812022651_add_access_pattern_indexes.sql

It will be applied to the production database automatically when this PR merges, before the new lambda code is deployed. Please confirm before requesting review:

  • Applied and tested locally. cd apps/backend && make migrate, then run the affected lambda's tests (cd apps/backend/lambdas/<name> && npm test). make show-migrations shows what applied.
  • Safe for the code that is live right now. During the deploy window -- and indefinitely if the deploy fails -- the currently deployed lambdas run against your new schema. Additive changes (CREATE TABLE, nullable ADD COLUMN, CREATE INDEX) are fine in one PR. DROP COLUMN, renames, ADD COLUMN NOT NULL with no default, and new UNIQUE/CHECK/FOREIGN KEY constraints need two merged PRs -- see the expand/contract rules in apps/backend/db/README.md.
  • Kept separate from unrelated changes. A migration PR should ideally contain the migration, the code that needs it, and nothing else. It changes production state, it is the one thing here that redeploying cannot roll back, and a reviewer should be able to see the whole schema change without scrolling past unrelated work.
  • No already-merged migration was edited. Fix an old migration by adding a new one; there is no down.

shared/types/db-types.d.ts is regenerated and pushed to this branch automatically -- don't hand-edit it. Expect one red migrations-fresh check before that commit lands.

github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibah nourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Database Types Check Complete

The database schema files were modified, but the regenerated TypeScript types are identical to the existing ones.

No changes were needed and the type definitions are already up to date.

Both lessons from the index audit: aggregating in the lambda costs a full
scan no index can fix, and new filter/join/sort columns need an index
because Postgres does not create them for foreign keys.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Database Types Check Complete

The database schema files were modified, but the regenerated TypeScript types are identical to the existing ones.

No changes were needed and the type definitions are already up to date.

@nourshoreibah
nourshoreibah merged commit acf2ba2 into main Aug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-review The PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant