-
Notifications
You must be signed in to change notification settings - Fork 10.2k
fix: add backfill migration for routing response denormalized tables #21474
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
This PR is being marked as stale due to inactivity. |
a48b1c5
to
ccef367
Compare
Note to myself: To skip these migrations: insert into
_prisma_migrations (
id,
migration_name,
checksum,
started_at,
finished_at,
applied_steps_count
)
values (
gen_random_uuid(),
'20250618093846_routing_form_response_denormalize_backfill',
'<INSERT-CHECKSUM1>',
now(),
now(),
1
),
(
gen_random_uuid(),
'20250618093923_routing_form_response_field_backfill',
'<INSERT-CHECKSUM2>',
now(),
now(),
1
); CHECKSUM1 ↓ sha256sum packages/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql CHECKSUM2 ↓ sha256sum packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql |
This PR is being marked as stale due to inactivity. |
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.
Will be released as part of tonights release.
WalkthroughThis pull request adds two standalone SQL migration scripts for backfilling denormalized data related to routing form responses. The first script performs a batch upsert on the "RoutingFormResponseDenormalized" table by processing records in chunks of 1000 from the "App_RoutingForms_FormResponse" table, joining with related tables, and inserting or updating records as needed while logging progress and throttling load. The second script processes the same source table in chunks, identifying routing form response fields requiring updates by comparing existing denormalized data with expected values derived from JSON responses and field definitions, then calls a reprocessing function for each such response, handling errors and logging progress similarly. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (3)
packages/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql (2)
4-8
: Consider derivingstart_id
from the data instead of hard-coding1
.
If the smallestApp_RoutingForms_FormResponse.id
is greater than 1, the first chunk does an unnecessary DELETE on a range that cannot hold rows, slightly inflatingprocessed_count
and wasting time.-start_id INTEGER := 1; -- Starting ID +SELECT COALESCE(MIN(id), 1) INTO start_id FROM "App_RoutingForms_FormResponse";
23-29
:processed_count
is over-counted when gaps exist in IDs.
processed_count := processed_count + chunk_size;
assumes every BETWEEN range contains exactlychunk_size
responses, which is false when IDs are sparse or at the tail of the table. Prefer COUNT(*) of the actually inserted rows (or useGET DIAGNOSTICS
) to keep the progress log trustworthy.packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql (1)
30-33
:WHEN OTHERS
swallows useful detail; include SQLSTATE.
CapturingSQLSTATE
helps triage repeated failures.-RAISE WARNING 'Failed to process responseId %: %', response_record.id, SQLERRM; +RAISE WARNING 'Failed responseId % (SQLSTATE %, message %)', + response_record.id, SQLSTATE, SQLERRM;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql
(1 hunks)packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql
(1 hunks)
🔇 Additional comments (1)
packages/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql (1)
58-99
: LEFT JOINs can duplicate the targetid
, riskingunique_violation
.
If a booking has multipleTracking
rows, the SELECT will return >1 row for the samer.id
, breaking the presumed PK onRoutingFormResponseDenormalized.id
. AddDISTINCT ON (r.id)
or aggregate the tracking columns (e.g.,MAX
) before insertion.
...es/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql
Outdated
Show resolved
Hide resolved
...es/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql
Outdated
Show resolved
Hide resolved
Graphite Automations"Add ready-for-e2e label" took an action on this PR • (07/28/25)1 label was added to this PR based on Keith Williams's automation. "Add consumer team as reviewer" took an action on this PR • (07/28/25)1 reviewer was added to this PR based on Keith Williams's automation. |
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: 1
♻️ Duplicate comments (3)
packages/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql (2)
1-26
: Entire backfill still runs in one monolithic transactionThe DO block is a single implicit transaction, so every DELETE/UPSERT lock it takes is held until the very end. On large tables this can block production traffic, create bloat, and make the migration nearly un-cancelable. Please commit per chunk (e.g. move the chunk logic to a helper function invoked with
PERFORM …; COMMIT;
) or use autonomous transactions (CALL
) instead.
194-195
:pg_sleep
executed while locks are heldBecause no commit happens before the sleep, row/share locks stay active for the whole second, defeating the throttling purpose. Move the sleep after a per-chunk commit or drop it.
packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql (1)
124-126
:pg_sleep
holds locks for the entire pauseWithout a preceding commit the sleep simply lengthens the lock window. Commit (or split into separate transactions) before calling
pg_sleep
, or delete the sleep.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/prisma/migrations/20250618093846_routing_form_response_denormalize_backfill/migration.sql
(1 hunks)packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql
(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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
packages/prisma/migrations/20250618093923_routing_form_response_field_backfill/migration.sql
Outdated
Show resolved
Hide resolved
e59fc85
to
a989026
Compare
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
LGTM, prod is prepared. 👍
What does this PR do?
This PR adds backfill migration for the
RoutingFormResponseDenormalized
andRoutingFormResponseField
tables.Mandatory Tasks (DO NOT REMOVE)