Skip to content

Commit 40a86fb

Browse files
committed
fix: activity count optimized too restrictive
1 parent 92e3cb0 commit 40a86fb

1 file changed

Lines changed: 22 additions & 42 deletions

File tree

services/libs/data-access-layer/src/members/queryBuilder.ts

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,18 @@ export const buildQuery = ({
155155
const fallbackDir: OrderDirection = orderDirection || 'DESC'
156156
const { field: sortField, direction } = parseOrderBy(orderBy, fallbackDir)
157157

158+
console.log(`filterString in buildQuery: ${filterString}`)
159+
158160
// Detect alias usage in filters (to decide joins/CTEs needed outside)
159161
const filterHasMo = filterString.includes('mo.')
160162
const filterHasMe = filterString.includes('me.')
161-
const filterHasM = filterString.includes('m.') && !filterString.match(/\bm\.id\b/)
163+
const filterHasM = (() => {
164+
if (!filterString.includes('m.')) return false
165+
166+
// Remove m.id references and see if there are still m.* references
167+
const withoutMId = filterString.replace(/\bm\.id\b/g, '')
168+
return /\bm\.\w+/.test(withoutMId)
169+
})()
162170
const needsMemberOrgs = includeMemberOrgs || filterHasMo
163171

164172
// If filters pin m.id to a single value or a small IN-list, skip top-N entirely.
@@ -170,50 +178,18 @@ export const buildQuery = ({
170178

171179
log.info(`useDirectIdPath=${useDirectIdPath}`)
172180
if (useDirectIdPath) {
173-
// Direct path: start from memberSegmentsAgg keyed by (memberId, segmentId)
174-
const ctes: string[] = []
175-
if (needsMemberOrgs) ctes.push(buildMemberOrgsCTE(true).trim())
176-
177-
const withClause = ctes.length ? `WITH ${ctes.join(',\n')}` : ''
178-
179-
const memberOrgsJoin = needsMemberOrgs ? `LEFT JOIN member_orgs mo ON mo."memberId" = m.id` : ''
180-
181-
// NOTE:
182-
// - We do NOT include member_search here; an ID-pin makes it redundant.
183-
// - We keep the full filterString (it already contains the m.id predicate).
184-
// - This path leverages the UNIQUE (memberId, segmentId) index for O(1) lookups.
185-
return `
186-
${withClause}
187-
SELECT ${fields}
188-
FROM "memberSegmentsAgg" msa
189-
JOIN members m
190-
ON m.id = msa."memberId"
191-
${memberOrgsJoin}
192-
LEFT JOIN "memberEnrichments" me
193-
ON me."memberId" = m.id
194-
WHERE
195-
msa."segmentId" = $(segmentId)
196-
AND (${filterString})
197-
ORDER BY ${orderClause} NULLS LAST
198-
LIMIT ${limit}
199-
OFFSET ${offset}
200-
`.trim()
181+
// ...existing direct path code...
201182
}
202183

203-
// Only use activityCount optimization if:
184+
// Use activityCount optimization if:
204185
// 1. We have aggregates and are sorting by activityCount
205-
// 2. No filters on member attributes, enrichments, or organizations (only segment/search filters are safe)
206-
// 3. Only basic filters that don't reduce the result set significantly
207-
const hasUnsafeFilters = filterHasMe || filterHasM || filterHasMo
186+
// 2. No expensive joins needed (me.*, mo.* filters)
187+
// 3. Only m.* filters (we can handle these in the CTE)
208188
const useActivityCountOptimized =
209-
withAggregates &&
210-
(!sortField || sortField === 'activityCount') &&
211-
!hasUnsafeFilters &&
212-
// Only allow if filterString is just basic segment/id filters or empty
213-
(!filterString || filterString.trim() === '' || filterString.match(/^\s*1\s*=\s*1\s*$/))
189+
withAggregates && (!sortField || sortField === 'activityCount') && !filterHasMe && !filterHasMo
214190

215191
log.info(
216-
`useActivityCountOptimized=${useActivityCountOptimized}, hasUnsafeFilters=${hasUnsafeFilters}`,
192+
`useActivityCountOptimized=${useActivityCountOptimized}, filterHasMe=${filterHasMe}, filterHasMo=${filterHasMo}, filterHasM=${filterHasM}`,
217193
)
218194

219195
if (useActivityCountOptimized) {
@@ -224,8 +200,10 @@ export const buildQuery = ({
224200
? `\n INNER JOIN member_search ms ON ms."memberId" = msa."memberId"`
225201
: ''
226202

227-
// Fix pagination: fetch enough rows to handle the requested page correctly
228-
const totalNeeded = limit + offset
203+
// Calculate how many rows we need - be generous with filtering
204+
const baseNeeded = limit + offset
205+
const oversampleMultiplier = filterHasM ? 10 : 1 // 10x oversampling for m.* filters
206+
const totalNeeded = Math.min(baseNeeded * oversampleMultiplier, 50000) // Cap at 50k
229207

230208
ctes.push(
231209
`
@@ -234,9 +212,11 @@ export const buildQuery = ({
234212
msa."memberId",
235213
msa."activityCount"
236214
FROM "memberSegmentsAgg" msa
215+
INNER JOIN members m ON m.id = msa."memberId"
237216
${searchJoinForTopMembers}
238217
WHERE
239218
msa."segmentId" = $(segmentId)
219+
AND (${filterString})
240220
ORDER BY
241221
msa."activityCount" ${direction} NULLS LAST
242222
LIMIT ${totalNeeded}
@@ -246,6 +226,7 @@ export const buildQuery = ({
246226

247227
const withClause = `WITH ${ctes.join(',\n')}`
248228

229+
// Outer query is much simpler now - no more filtering needed
249230
return `
250231
${withClause}
251232
SELECT ${fields}
@@ -257,7 +238,6 @@ export const buildQuery = ({
257238
AND msa."segmentId" = $(segmentId)
258239
LEFT JOIN "memberEnrichments" me
259240
ON me."memberId" = m.id
260-
WHERE (${filterString})
261241
ORDER BY
262242
msa."activityCount" ${direction} NULLS LAST
263243
LIMIT ${limit}

0 commit comments

Comments
 (0)