Skip to content

Comments

feat: add index [startTime, endTime] for BookingDenormalized#23944

Merged
eunjae-lee merged 2 commits intomainfrom
fix/add-index-for-booking-denormalized
Sep 19, 2025
Merged

feat: add index [startTime, endTime] for BookingDenormalized#23944
eunjae-lee merged 2 commits intomainfrom
fix/add-index-for-booking-denormalized

Conversation

@eunjae-lee
Copy link
Contributor

What does this PR do?

In #23752, we introduced a new date range filter (startTime ~ endTime) that can be used instead of createdAt. However, we didn't have the [startTime, endTime] index.

@eunjae-lee eunjae-lee requested a review from a team as a code owner September 19, 2025 12:50
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 19, 2025

Walkthrough

A new composite index on (startTime, endTime) is introduced for the BookingDenormalized table. The Prisma migration creates the PostgreSQL index BookingDenormalized_startTime_endTime_idx via CREATE INDEX. The Prisma schema is updated to include @@index([startTime, endTime]) on the BookingDenormalized model, in addition to existing indices on status, teamId + isTeamBooking, and userId + isTeamBooking. No data schema fields or constraints are modified beyond this index. No exported/public entity signatures are changed.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately describes the primary change: adding a composite index on [startTime, endTime] to the BookingDenormalized model/table. It is concise, specific, and directly reflects the migration and Prisma schema updates in the changeset. A reviewer scanning PR history will understand the main intent from this single sentence.
Description Check ✅ Passed The PR description explains why the index is being added and references PR #23752 and the new date-range filter (startTime ~ endTime), which directly relates to the migration and schema changes in the changeset. It is not off-topic and therefore satisfies the lenient description check. The description is brief but sufficiently connected to the changes being made.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/add-index-for-booking-denormalized

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the ❗️ migrations contains migration files label Sep 19, 2025
@graphite-app graphite-app bot requested a review from a team September 19, 2025 12:50
@keithwillcode keithwillcode added consumer core area: core, team members only labels Sep 19, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
packages/prisma/schema.prisma (2)

1746-1746: Name the index explicitly to avoid churn across re-introspections.

Apply:

-  @@index([startTime, endTime])
+  @@index([startTime, endTime], name: "BookingDenormalized_startTime_endTime_idx")

1746-1746: Consider workload-aligned composites (optional).

If common filters also include status or teamId/isTeamBooking, (status, startTime) or (teamId, isTeamBooking, startTime) may yield better selectivity than (startTime, endTime). Validate with query plans before adding.

packages/prisma/migrations/20250919124540_booking_denormalized_starttime_endtime/migration.sql (1)

1-2: Avoid blocking writes when indexing large tables.

CREATE INDEX without CONCURRENTLY will lock writes for the duration. Prisma migrations run in a transaction and can’t use CONCURRENTLY; for prod, create the index manually first with CONCURRENTLY and keep this migration as a no‑op (or guarded with IF NOT EXISTS).

Proposed ops steps (run manually in prod):

-- Step 1 (safe): 
CREATE INDEX CONCURRENTLY IF NOT EXISTS "BookingDenormalized_startTime_endTime_idx"
ON "BookingDenormalized"("startTime", "endTime");

Then, either:

  • Leave this migration as-is for dev/test, or
  • Change it to IF NOT EXISTS and execute outside a transaction in a one-off SQL deployment (not via Prisma) for prod environments.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between dfdda8a and 3482940.

📒 Files selected for processing (2)
  • packages/prisma/migrations/20250919124540_booking_denormalized_starttime_endtime/migration.sql (1 hunks)
  • packages/prisma/schema.prisma (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.
Learnt from: eunjae-lee
PR: calcom/cal.com#23752
File: packages/lib/server/service/InsightsBookingBaseService.ts:340-371
Timestamp: 2025-09-18T08:50:49.906Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), the startTime date range filter intentionally uses different fields for the lower and upper bounds: `startDate <= "startTime" AND "endTime" <= endDate`. This ensures bookings start within the selected range but don't extend beyond the end date, which is the intended business logic and not an inconsistency.
📚 Learning: 2025-08-19T09:47:49.478Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.

Applied to files:

  • packages/prisma/migrations/20250919124540_booking_denormalized_starttime_endtime/migration.sql
  • packages/prisma/schema.prisma
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Type check / check-types
  • GitHub Check: Linters / lint
  • GitHub Check: Tests / Unit
🔇 Additional comments (1)
packages/prisma/schema.prisma (1)

1746-1746: Verify composite index (startTime, endTime) is beneficial

A btree composite on (startTime, endTime) often only uses the leftmost column; Postgres can BitmapAnd single‑column indexes on startTime and endTime, making the composite possibly redundant. EXPLAIN (ANALYZE, BUFFERS) the actual Insights query to confirm the composite index is chosen and reduces cost/IO. Example (run on staging and paste the plan):

SET enable_seqscan=off;
EXPLAIN (ANALYZE, BUFFERS)
SELECT id
FROM "BookingDenormalized"
WHERE "startTime" >= TIMESTAMPTZ '2025-01-01'
AND "endTime" <= TIMESTAMPTZ '2025-12-31';

Location: packages/prisma/schema.prisma:1746.

@vercel
Copy link

vercel bot commented Sep 19, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 19, 2025 2:30pm
cal-eu Ignored Ignored Sep 19, 2025 2:30pm

@github-actions
Copy link
Contributor

E2E results are ready!

@eunjae-lee eunjae-lee merged commit 6ee752a into main Sep 19, 2025
63 of 65 checks passed
@eunjae-lee eunjae-lee deleted the fix/add-index-for-booking-denormalized branch September 19, 2025 15:21
emrysal added a commit that referenced this pull request Sep 19, 2025
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only ❗️ migrations contains migration files ready-for-e2e size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants