perf(spp_programs): add composite indexes for frequent query patterns#118
perf(spp_programs): add composite indexes for frequent query patterns#118kneckinator wants to merge 1 commit into19.0from
Conversation
Add indexes on spp_entitlement(cycle_id, partner_id), spp_entitlement(cycle_id, state), and spp_program_membership(program_id, state) to avoid sequential scans during prepare_entitlements duplicate checks, cycle computed fields, and beneficiary pagination.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the performance of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces three composite database indexes to improve query performance for frequently accessed data patterns in spp_programs. The indexes are created idempotently within the models' init methods, and new tests are included to verify their existence. The implementation for creating the indexes is sound. However, the accompanying tests could be more robust. I've provided suggestions to make the tests more specific by checking for the exact index names and column order, which will improve their reliability and prevent potential future regressions.
| AND indexdef LIKE '%%cycle_id%%' | ||
| AND indexdef LIKE '%%partner_id%%' |
There was a problem hiding this comment.
The current query to check for the index is not specific enough. It checks for the presence of cycle_id and partner_id in the index definition but doesn't enforce that they are the only columns or their specific order. A more robust test would check for the exact index name and column order.
| AND indexdef LIKE '%%cycle_id%%' | |
| AND indexdef LIKE '%%partner_id%%' | |
| AND indexname = 'spp_entitlement_cycle_id_partner_id_idx' | |
| AND indexdef LIKE '%(cycle_id, partner_id)%' |
| AND indexdef LIKE '%%cycle_id%%' | ||
| AND indexdef LIKE '%%state%%' |
There was a problem hiding this comment.
The query to verify the index's existence is too broad. It only checks that cycle_id and state are mentioned in the index definition but doesn't validate the column order or confirm it's a composite index specifically on these two columns. This could lead to false positives.
| AND indexdef LIKE '%%cycle_id%%' | |
| AND indexdef LIKE '%%state%%' | |
| AND indexname = 'spp_entitlement_cycle_id_state_idx' | |
| AND indexdef LIKE '%(cycle_id, state)%' |
| AND indexdef LIKE '%%program_id%%' | ||
| AND indexdef LIKE '%%state%%' |
There was a problem hiding this comment.
The test for the program membership index is not specific enough. It confirms that program_id and state are in the index definition but doesn't check their order or if they are the only columns. This could allow an incorrectly defined index to pass the test.
| AND indexdef LIKE '%%program_id%%' | |
| AND indexdef LIKE '%%state%%' | |
| AND indexname = 'spp_program_membership_program_id_state_idx' | |
| AND indexdef LIKE '%(program_id, state)%' |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #118 +/- ##
==========================================
- Coverage 70.14% 69.94% -0.21%
==========================================
Files 739 783 +44
Lines 43997 46604 +2607
==========================================
+ Hits 30863 32595 +1732
- Misses 13134 14009 +875
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
1 participant
Summary
Add composite database indexes for the three most frequent query patterns in
spp_programs. No version bump needed — indexes are created idempotently viainit().Changes
spp_entitlement_cycle_id_partner_id_idxspp_entitlement(cycle_id, partner_id)prepare_entitlementsduplicate check searches by these columns on every batchspp_entitlement_cycle_id_state_idxspp_entitlement(cycle_id, state)total_amount,show_approve_button,entitlements_count) filter by cycle + statespp_program_membership_program_id_state_idxspp_program_membership(program_id, state)get_beneficiaries()andcount_beneficiaries()filter by program + state on every async batch dispatchIndexes are created via
init()withCREATE INDEX IF NOT EXISTS, so they're safe to apply on existing databases without migration.Context
Phase 2 of 9 in the
spp_programsperformance optimization effort. Independent of Phase 1 (SQL constraints).Test Plan
./scripts/test_single_module.sh spp_programspasses