-
-
Notifications
You must be signed in to change notification settings - Fork 821
perf(webapp): Add index to speedup waitpoint tokens dashboard list query #2498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughAdds a Prisma schema change to define an index on the Waitpoint model across environmentId, type, and id in descending order, preceded by a documentation comment. Introduces a corresponding SQL migration that creates the index concurrently on public.Waitpoint with the same column ordering, using CREATE INDEX CONCURRENTLY IF NOT EXISTS. No other models, fields, or exported entities are modified. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Small scope with two files; straightforward index addition mirrored between Prisma schema and SQL migration. Low logic density and low heterogeneity; primary checks involve column order, sort direction, naming consistency, and concurrent/IF NOT EXISTS usage. Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
internal-packages/database/prisma/migrations/20250912153951_add_waitpoint_dashboard_index/migration.sql (1)
2-2
: Optional: validate idx usage with the dashboard query.If the query filters by environmentId and type and orders by id DESC with LIMIT, this index should be picked. If the query also filters on status or a time window predominantly, consider whether an alternate or partial index would outperform.
Would you like me to draft EXPLAIN variants for the common query shapes you expect?
internal-packages/database/prisma/schema.prisma (2)
1065-1067
: Pin the DB index name to avoid drift.Prisma’s default name matches your SQL migration today, but future renames/truncation could diverge. Add map to lock it to Waitpoint_environmentId_type_id_idx.
- /// For the waitpoint token dashboard page - @@index([environmentId, type, id(sort: Desc)]) + /// For the waitpoint token dashboard page + @@index([environmentId, type, id(sort: Desc)], map: "Waitpoint_environmentId_type_id_idx")
1065-1067
: Sanity-check the sort key vs. access pattern.If the dashboard relies on recent items, createdAt DESC is typically more correlated with recency than cuid-based id DESC. If you’re paginating by id cursor specifically, current choice is correct; otherwise consider a createdAt-based index.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal-packages/database/prisma/migrations/20250912153951_add_waitpoint_dashboard_index/migration.sql
(1 hunks)internal-packages/database/prisma/schema.prisma
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
internal-packages/database/prisma/migrations/20250912153951_add_waitpoint_dashboard_index/migration.sql (2)
2-2
: Good index choice and concurrent creation.Name, column order, and DESC on id line up with the stated dashboard query and minimize write blocking via CONCURRENTLY. LGTM.
2-2
: Confirm non-transactional application of this migration.CREATE INDEX CONCURRENTLY cannot run inside a transaction. Ensure Prisma applies this migration in a non-transactional mode; otherwise deploys will fail. Please verify on your CI/staging run logs.
No description provided.