fix: add new backfill script for RoutingFormResponseDenormalized #23166
Conversation
WalkthroughAdds a SQL migration at packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql to backfill RoutingFormResponseDenormalized from App_RoutingForms_FormResponse and related tables in 1,000-ID chunks. It computes end_id and total_count, exits if empty, assembles expected data via joins (Form, Booking, users, EventType, Tracking, assignment reason subquery), detects missing/changed rows using IS DISTINCT FROM (including event type scheduling type as text), and upserts via INSERT ... ON CONFLICT (id) DO UPDATE. It tracks per-chunk ROW_COUNT with GET DIAGNOSTICS, accumulates processed counts, and emits notices. Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/18/25)1 reviewer was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql (5)
47-53: Make AssignmentReason selection deterministic.LIMIT 1 without ORDER BY is nondeterministic when multiple reasons exist. Pick a stable ordering (e.g., latest).
Apply:
- SELECT ar."reasonString" - FROM "AssignmentReason" ar - WHERE ar."bookingId" = b.id - LIMIT 1 + SELECT ar."reasonString" + FROM "AssignmentReason" ar + WHERE ar."bookingId" = b.id + ORDER BY ar."createdAt" DESC NULLS LAST, ar.id DESC + LIMIT 1
68-68: Potential row duplication via Tracking join; constrain to a single row.If multiple Tracking rows exist per booking, this LEFT JOIN can duplicate expected_data rows, causing redundant work (and potentially ON CONFLICT churn). Use a lateral subquery to pick one deterministic row.
Apply:
- LEFT JOIN "Tracking" t ON t."bookingId" = b.id + LEFT JOIN LATERAL ( + SELECT t1.* + FROM "Tracking" t1 + WHERE t1."bookingId" = b.id + ORDER BY t1."createdAt" DESC NULLS LAST, t1.id DESC + LIMIT 1 + ) t ON TRUE
37-37: Guard calculate_booking_status_order against NULL status (defensive).If b.status is NULL and calculate_booking_status_order is not STRICT, it may error. A CASE wrapper avoids surprises.
Apply:
- calculate_booking_status_order(b.status::text) as expected_booking_status_order, + CASE + WHEN b.status IS NULL THEN NULL + ELSE calculate_booking_status_order(b.status::text) + END as expected_booking_status_order,
4-6: Optional: start chunking at MIN(id) to skip empty ranges.Starting at 1 can loop over empty gaps if IDs are sparse or started higher. Compute start_id from MIN(id).
Apply:
- start_id INTEGER := 1; -- Starting ID + start_id INTEGER := 1; -- Starting ID (will be overridden by MIN(id) if present) @@ - SELECT COALESCE(MAX(id), 0) INTO end_id FROM "App_RoutingForms_FormResponse"; - SELECT COUNT(*) INTO total_count FROM "App_RoutingForms_FormResponse"; + SELECT COALESCE(MAX(id), 0) INTO end_id FROM "App_RoutingForms_FormResponse"; + SELECT COUNT(*), COALESCE(MIN(id), 1) INTO total_count, start_id FROM "App_RoutingForms_FormResponse"; @@ - FOR current_id IN SELECT * FROM generate_series(start_id, end_id, chunk_size) + FOR current_id IN SELECT * FROM generate_series(start_id, end_id, chunk_size) @@ - WHERE r.id BETWEEN current_id AND current_id + chunk_size - 1 + WHERE r.id BETWEEN current_id AND current_id + chunk_size - 1(Only the MIN(id) addition changes behavior; other lines are shown for context.)
Also applies to: 12-15, 24-25, 69-70
95-95: Minor: redundant ::text cast on an already-text expression.expected_event_type_scheduling_type is already text; the extra ::text on Line 153 is harmless but unnecessary. Feel free to keep or drop for consistency.
Apply:
- r.expected_event_type_scheduling_type::text, + r.expected_event_type_scheduling_type,Also applies to: 153-153
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/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
🔇 Additional comments (3)
packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql (3)
66-66: Corrected JOIN to users via Booking.userId looks right.This aligns with the PR objective: LEFT JOIN "users" u ON b."userId" = u.id. Good catch fixing the previous incorrect join via form.userId.
24-26: Upsert/filtering strategy looks solid.
- Using IS DISTINCT FROM avoids NULL pitfalls.
- records_to_update filters out unchanged rows, minimizing writes.
- ON CONFLICT (id) DO UPDATE keeps it idempotent.
- Chunk notices + ROW_COUNT accounting are clear.
Also applies to: 105-187, 188-196
37-37: calculate_booking_status_order verified and safe for NULL input
The function is defined in migrations
- 20250502113828_setup_triggers_for_routing_form_response_denormalized
- 20250711154030_setup_triggers_for_routing_form_response_denormalized_2
and, being non-STRICT, will execute with a NULL argument and hit theELSE 999branch. No changes required.
.../prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql
Show resolved
Hide resolved
E2E results are ready! |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
emrysal
left a comment
emrysal
left a comment
There was a problem hiding this comment.
Approved, already ran on Production.
What does this PR do?
This is a follow-up of #21474. The #21474 PR had a bad join.
This is the diff between #21474 and this.
Fortunately, the trigger is correctly implemented:
packages/prisma/migrations/20250711154030_setup_triggers_for_routing_form_response_denormalized_2/migration.sql