fix: optimize bookings/get query to use subqueries instead of materializing 400+ parameters (#27319)
PERFORMANCE: The bookings/get endpoint was taking 24s due to materializing 400+ email addresses as individual parameters in WHERE IN clauses. This caused massive UNION queries that overwhelmed the database. Changes: - Replace IN clauses with Kysely subqueries for queries 4, 5, 6, and 7 - Query 4 & 5: Use subquery to get emails from users via Membership table - Query 6: Use subquery to get event type IDs from EventType table - Query 7: Use subquery to get user IDs from Membership table - Only fetch user IDs when needed for userIds filter validation - Add new getUserIdsFromTeamIds function (lighter version without emails) Before: WHERE email IN ($1, $2, ..., $400+) - passes 400+ parameters After: WHERE email IN (SELECT email FROM users JOIN Membership...) - passes only team IDs This reduces parameter count from 400+ to typically 1-10 team IDs, letting the database optimize the query execution plan. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Devin AI
parent
cba329d6d2
commit
718aef579c
@@ -335,11 +335,10 @@ describe("getBookings - PBAC Permission Checks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("Event type filtering", () => {
|
||||
it("should get event types from teams where user has booking.read permission", async () => {
|
||||
describe("Event type filtering with subqueries", () => {
|
||||
it("should use subqueries for event types from teams where user has booking.read permission", async () => {
|
||||
mockGetTeamIdsWithPermission.mockResolvedValue([1]);
|
||||
mockPrisma.user.findMany = vi.fn().mockResolvedValue([]);
|
||||
mockPrisma.$queryRaw = vi.fn().mockResolvedValue([{ id: 10 }, { id: 11 }]);
|
||||
mockPrisma.booking.groupBy = vi.fn().mockResolvedValue([]);
|
||||
|
||||
await getBookings({
|
||||
@@ -352,13 +351,15 @@ describe("getBookings - PBAC Permission Checks", () => {
|
||||
skip: 0,
|
||||
});
|
||||
|
||||
// Verify event types are fetched from team 1 using raw SQL query
|
||||
expect(mockPrisma.$queryRaw).toHaveBeenCalled();
|
||||
// PERFORMANCE: With the optimization, we no longer call $queryRaw to fetch event type IDs.
|
||||
// Instead, we use subqueries directly in the SQL to avoid materializing large arrays.
|
||||
// The query is built using Kysely and executed via executeQuery.
|
||||
expect(mockKysely.executeQuery).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("User IDs and emails retrieval", () => {
|
||||
it("should get user IDs and emails from teams with booking permission", async () => {
|
||||
describe("User IDs retrieval for permission validation", () => {
|
||||
it("should only fetch user IDs when userIds filter is provided", async () => {
|
||||
mockGetTeamIdsWithPermission.mockResolvedValue([1, 2]);
|
||||
mockPrisma.user.findMany = vi.fn().mockResolvedValue([
|
||||
{ id: 2, email: "user2@example.com" },
|
||||
@@ -372,13 +373,38 @@ describe("getBookings - PBAC Permission Checks", () => {
|
||||
prisma: mockPrisma,
|
||||
kysely: mockKysely,
|
||||
bookingListingByStatus: ["upcoming"],
|
||||
filters: {},
|
||||
filters: {
|
||||
userIds: [2], // When userIds filter is provided, we need to validate permissions
|
||||
},
|
||||
take: 10,
|
||||
skip: 0,
|
||||
});
|
||||
|
||||
// Verify users are fetched from teams 1 and 2
|
||||
// Verify users are fetched from teams 1 and 2 for permission validation
|
||||
expect(mockPrisma.user.findMany).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should NOT fetch user IDs when no userIds filter is provided (uses subqueries instead)", async () => {
|
||||
mockGetTeamIdsWithPermission.mockResolvedValue([1, 2]);
|
||||
// Reset the mock to track calls
|
||||
mockPrisma.user.findMany = vi.fn().mockResolvedValue([]);
|
||||
mockPrisma.eventType.findMany = vi.fn().mockResolvedValue([]);
|
||||
mockPrisma.booking.groupBy = vi.fn().mockResolvedValue([]);
|
||||
|
||||
await getBookings({
|
||||
user: mockUser,
|
||||
prisma: mockPrisma,
|
||||
kysely: mockKysely,
|
||||
bookingListingByStatus: ["upcoming"],
|
||||
filters: {}, // No userIds filter
|
||||
take: 10,
|
||||
skip: 0,
|
||||
});
|
||||
|
||||
// PERFORMANCE: Without userIds filter, we don't need to fetch user IDs for validation.
|
||||
// The query uses subqueries instead of materializing all user IDs/emails.
|
||||
// user.findMany should NOT be called for getUserIdsFromTeamIds
|
||||
expect(mockPrisma.user.findMany).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -118,25 +118,25 @@ export async function getBookings({
|
||||
orgId: user.orgId ?? undefined,
|
||||
});
|
||||
|
||||
// Only fetch user IDs from teams if we need to validate userIds filter
|
||||
// PERFORMANCE: We no longer need to fetch all emails/IDs for the main query since we use subqueries
|
||||
const needsUserIdsValidation = !!filters?.userIds && filters.userIds.length > 0;
|
||||
|
||||
const [
|
||||
eventTypeIdsFromTeamIdsFilter,
|
||||
attendeeEmailsFromUserIdsFilter,
|
||||
eventTypeIdsFromEventTypeIdsFilter,
|
||||
eventTypeIdsWhereUserHasBookingPermission,
|
||||
userIdsAndEmailsWhereUserHasBookingPermission,
|
||||
allAccessibleUserIds,
|
||||
] = await Promise.all([
|
||||
getEventTypeIdsFromTeamIdsFilter(prisma, filters?.teamIds),
|
||||
getAttendeeEmailsFromUserIdsFilter(prisma, user.email, filters?.userIds),
|
||||
getEventTypeIdsFromEventTypeIdsFilter(prisma, filters?.eventTypeIds),
|
||||
getEventTypeIdsFromTeamIdsFilter(prisma, teamIdsWithBookingPermission),
|
||||
getUserIdsAndEmailsFromTeamIds(prisma, teamIdsWithBookingPermission),
|
||||
// Only fetch accessible user IDs when we need to validate the userIds filter
|
||||
needsUserIdsValidation ? getUserIdsFromTeamIds(prisma, teamIdsWithBookingPermission) : Promise.resolve([]),
|
||||
]);
|
||||
|
||||
const bookingQueries: { query: BookingsUnionQuery; tables: (keyof DB)[] }[] = [];
|
||||
|
||||
// Get user IDs and emails from teams where user has booking permission
|
||||
const [allAccessibleUserIds, allAccessibleUserEmails] = userIdsAndEmailsWhereUserHasBookingPermission;
|
||||
|
||||
// If userIds filter is provided
|
||||
if (!!filters?.userIds && filters.userIds.length > 0) {
|
||||
const areUserIdsWithinUserOrgOrTeam = filters.userIds.every((userId) =>
|
||||
@@ -242,7 +242,8 @@ export async function getBookings({
|
||||
});
|
||||
// 4. Scope depends on `user.orgId`:
|
||||
// - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get bookings where organization/team members are attendees
|
||||
if (allAccessibleUserEmails?.length) {
|
||||
// PERFORMANCE: Use subquery with team membership instead of materializing all emails (can be 400+ for large orgs)
|
||||
if (teamIdsWithBookingPermission?.length) {
|
||||
bookingQueries.push({
|
||||
query: kysely
|
||||
.selectFrom("Booking")
|
||||
@@ -252,13 +253,20 @@ export async function getBookings({
|
||||
.select("Booking.createdAt")
|
||||
.select("Booking.updatedAt")
|
||||
.innerJoin("Attendee", "Attendee.bookingId", "Booking.id")
|
||||
.where("Attendee.email", "in", allAccessibleUserEmails),
|
||||
.where("Attendee.email", "in", (eb) =>
|
||||
eb
|
||||
.selectFrom("users")
|
||||
.select("users.email")
|
||||
.innerJoin("Membership", "Membership.userId", "users.id")
|
||||
.where("Membership.teamId", "in", teamIdsWithBookingPermission)
|
||||
),
|
||||
tables: ["Booking", "Attendee"],
|
||||
});
|
||||
}
|
||||
// 5. Scope depends on `user.orgId`:
|
||||
// - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get bookings where organization/team members are attendees via seatsReference
|
||||
if (allAccessibleUserEmails?.length) {
|
||||
// PERFORMANCE: Use subquery with team membership instead of materializing all emails
|
||||
if (teamIdsWithBookingPermission?.length) {
|
||||
bookingQueries.push({
|
||||
query: kysely
|
||||
.selectFrom("Booking")
|
||||
@@ -269,14 +277,21 @@ export async function getBookings({
|
||||
.select("Booking.updatedAt")
|
||||
.innerJoin("Attendee", "Attendee.bookingId", "Booking.id")
|
||||
.innerJoin("BookingSeat", "Attendee.id", "BookingSeat.attendeeId")
|
||||
.where("Attendee.email", "in", allAccessibleUserEmails),
|
||||
.where("Attendee.email", "in", (eb) =>
|
||||
eb
|
||||
.selectFrom("users")
|
||||
.select("users.email")
|
||||
.innerJoin("Membership", "Membership.userId", "users.id")
|
||||
.where("Membership.teamId", "in", teamIdsWithBookingPermission)
|
||||
),
|
||||
tables: ["Booking", "Attendee", "BookingSeat"],
|
||||
});
|
||||
}
|
||||
|
||||
// 6. Scope depends on `user.orgId`:
|
||||
// - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get booking created for an event type within the organization/team
|
||||
if (eventTypeIdsWhereUserHasBookingPermission?.length) {
|
||||
// PERFORMANCE: Use subquery to get event type IDs instead of materializing them
|
||||
if (teamIdsWithBookingPermission?.length) {
|
||||
bookingQueries.push({
|
||||
query: kysely
|
||||
.selectFrom("Booking")
|
||||
@@ -285,15 +300,20 @@ export async function getBookings({
|
||||
.select("Booking.endTime")
|
||||
.select("Booking.createdAt")
|
||||
.select("Booking.updatedAt")
|
||||
.innerJoin("EventType", "EventType.id", "Booking.eventTypeId")
|
||||
.where("Booking.eventTypeId", "in", eventTypeIdsWhereUserHasBookingPermission),
|
||||
tables: ["Booking", "EventType"],
|
||||
.where("Booking.eventTypeId", "in", (eb) =>
|
||||
eb
|
||||
.selectFrom("EventType")
|
||||
.select("EventType.id")
|
||||
.where("EventType.teamId", "in", teamIdsWithBookingPermission)
|
||||
),
|
||||
tables: ["Booking"],
|
||||
});
|
||||
}
|
||||
|
||||
// 7. Scope depends on `user.orgId`:
|
||||
// - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get bookings created by users within the same organization/team
|
||||
if (allAccessibleUserIds?.length) {
|
||||
// PERFORMANCE: Use subquery with team membership instead of materializing all user IDs
|
||||
if (teamIdsWithBookingPermission?.length) {
|
||||
bookingQueries.push({
|
||||
query: kysely
|
||||
.selectFrom("Booking")
|
||||
@@ -302,7 +322,12 @@ export async function getBookings({
|
||||
.select("Booking.endTime")
|
||||
.select("Booking.createdAt")
|
||||
.select("Booking.updatedAt")
|
||||
.where("Booking.userId", "in", allAccessibleUserIds),
|
||||
.where("Booking.userId", "in", (eb) =>
|
||||
eb
|
||||
.selectFrom("Membership")
|
||||
.select("Membership.userId")
|
||||
.where("Membership.teamId", "in", teamIdsWithBookingPermission)
|
||||
),
|
||||
tables: ["Booking"],
|
||||
});
|
||||
}
|
||||
@@ -944,17 +969,15 @@ async function getEventTypeIdsFromEventTypeIdsFilter(prisma: PrismaClient, event
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets [IDs, Emails] of members from specified team IDs.
|
||||
* Gets user IDs of members from specified team IDs.
|
||||
* PERFORMANCE: This is a lighter version that only fetches IDs (not emails) for permission validation.
|
||||
* @param prisma The Prisma client.
|
||||
* @param teamIds Array of team IDs to get members from
|
||||
* @returns {Promise<[number[], string[]]>} [UserIDs, UserEmails] for members in the specified teams.
|
||||
* @returns {Promise<number[]>} UserIDs for members in the specified teams.
|
||||
*/
|
||||
async function getUserIdsAndEmailsFromTeamIds(
|
||||
prisma: PrismaClient,
|
||||
teamIds: number[]
|
||||
): Promise<[number[], string[]]> {
|
||||
async function getUserIdsFromTeamIds(prisma: PrismaClient, teamIds: number[]): Promise<number[]> {
|
||||
if (teamIds.length === 0) {
|
||||
return [[], []];
|
||||
return [];
|
||||
}
|
||||
|
||||
const users = await prisma.user.findMany({
|
||||
@@ -969,13 +992,9 @@ async function getUserIdsAndEmailsFromTeamIds(
|
||||
},
|
||||
select: {
|
||||
id: true,
|
||||
email: true,
|
||||
},
|
||||
});
|
||||
const userIds = Array.from(new Set(users.map((user) => user.id)));
|
||||
const userEmails = Array.from(new Set(users.map((user) => user.email)));
|
||||
|
||||
return [userIds, userEmails];
|
||||
return Array.from(new Set(users.map((user) => user.id)));
|
||||
}
|
||||
|
||||
function addStatusesQueryFilters(query: BookingsUnionQuery, statuses: InputByStatus[]) {
|
||||
|
||||
Reference in New Issue
Block a user