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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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 bothproject_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.

QueryBeforeAfter
Project-filtered expenditure page6846 buffers, 48.4 ms28 buffers, 0.8 ms
Unfiltered expenditure page6846 buffers, 23.8 ms28 buffers, 1.2 ms
GET /projects for a non-admin406 buffers, 6.2 ms34 buffers, 1.1 ms
Project-filtered report page230 buffers, 7.8 ms15 buffers, 2.3 ms
GET /projects/{id}/donors423 buffers, 7.4 ms82 buffers
DELETE one project (FK cascade)179.2 ms42.0 ms
DELETE one user (FK cascade)127.4 ms38.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-actionsBot added a commit that referenced this pull request Aug 12, 2026
@nourshoreibahnourshoreibah added the no-review The PR review bot won't run label Aug 12, 2026
github-actionsBot 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 mainAug 12, 2026
18 checks passed
@nourshoreibah
nourshoreibah deleted the worktree-db-indexes branch August 12, 2026 02:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-reviewThe PR review bot won't run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@nourshoreibah