Repository navigation
Serialize changemaker proposals in a single pass - #2493
Conversation
|
🔎 A preview deployment of this pull request is available at https://utilities.philanthropydatacommons.org/pdc-service-preview-2493-lvdq9/ (and upstream https://pdc-service-preview-2493-lvdq9.ondigitalocean.app) assuming this PR is still open. 🔍 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2493 +/- ##
=======================================
Coverage 93.73% 93.73%
=======================================
Files 312 312
Lines 3929 3929
Branches 532 510 -22
=======================================
Hits 3683 3683
Misses 245 245
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
587cb1f to
0d36ac2
Compare
| 'proposal', proposal, | ||
| 'createdAt', changemaker_proposal.created_at | ||
| ); | ||
| $$ LANGUAGE sql IMMUTABLE; |
There was a problem hiding this comment.
I love that postgresql has this capability (marking it IMMUTABLE).
| auth_context_is_administrator boolean DEFAULT FALSE | ||
| ) RETURNS TABLE (id int, object jsonb) AS $$ | ||
| WITH input_entries AS ( | ||
| SELECT cp.* |
There was a problem hiding this comment.
For a moment I thought "star: BAAAD, columns GOOOD" but that oversimplifies. Here we are building an intermediate result so it's OK. The higher abstraction function will filter.
| :changemakerId, | ||
| :proposalId | ||
| ) | ||
| RETURNING * |
There was a problem hiding this comment.
Again I had an internal knee-jerk reaction to * here but I think our pattern is to actually want all of them.
bickelj
left a comment
bickelj
left a comment
There was a problem hiding this comment.
Even though this is a smaller version of the previous similar changes, I did not dive so deep as to carefully understand each and every line, instead I'm leaning on our automated test passage and noticing that there were no changes to tests here.
I did some curl calls against the endpoint in demo vs the preview environment here and the preview here won in both absolute times and in consistency.
changemaker_proposal_to_json becomes a pure shape over an entry's already-serialized changemaker and proposal. build_changemakers_proposals_results gathers a whole page's changemakers and proposals in one call each to build_changemakers_results / build_proposals_results, replacing the per-row pair of builder calls the row-by-row serializer ran. The builder is volatile rather than STABLE: an entry embeds its proposal, which lists its changemakers from changemakers_proposals, so the insert path needs a fresh snapshot to see the row it just inserted. Issue #2492 Address GET /changemakerProposals slowness Issue #2475 Reconsider N+1 query approach for deep entities
0d36ac2 to
ab48eb1
Compare
This PR applies our new collection patterns (to remove
N+1queries) to theGET /changemakersProposalsendpoint.Resolves #2492