Skip to content

Commit 17e415d

Browse files
authored
fix(knowledge): read the projection-filled probe off the partial index instead of a sequential scan (#8139)
* fix(knowledge): read the projection-filled probe off the partial index instead of a sequential scan * test(knowledge): pin the backfill probe's ordered, capped shape
1 parent 06d0f7d commit 17e415d

4 files changed

Lines changed: 50 additions & 11 deletions

File tree

‎apps/sim/lib/knowledge/search/projection-source-acl-backfill.test.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,21 @@ describe('runProjectionSourceAclBackfill', () => {
7878

7979
it('analyzes and warms the projections on the same connection once both are filled, before closing it', async () => {
8080
await runProjectionSourceAclBackfill({})
81-
/** A row whose document is gone is not the fill's to finish; the probe joins the document. */
82-
expect(
83-
mockUnsafe.mock.calls.some(([query]) =>
84-
String(query).includes('JOIN document d ON d.id = s.document_id WHERE s.acl IS NULL')
81+
/**
82+
* A row whose document is gone is not the fill's to finish; the probe joins the document. It is
83+
* ordered and capped so only the unfilled-rows index can serve it: an `EXISTS` drops both and
84+
* leaves the planner a sequential scan of the projection.
85+
*/
86+
const probes = mockUnsafe.mock.calls
87+
.map(([query]) => String(query).replace(/\s+/g, ' '))
88+
.filter((query) => query.includes('AS unfilled'))
89+
expect(probes).toHaveLength(2)
90+
for (const probe of probes) {
91+
expect(probe).not.toContain('EXISTS')
92+
expect(probe).toContain(
93+
'JOIN document d ON d.id = s.document_id WHERE s.acl IS NULL ORDER BY s.id DESC LIMIT 1 ) IS NOT NULL AS unfilled'
8594
)
86-
).toBe(true)
95+
}
8796
expect(mockUnsafe.mock.calls.map(([query]) => query)).toEqual(
8897
expect.arrayContaining(['ANALYZE embedding_search', 'ANALYZE embedding_keyword_tin'])
8998
)

‎apps/sim/lib/knowledge/search/projection-source-acl-backfill.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,17 @@ export async function runProjectionSourceAclBackfill(
148148
/**
149149
* Whether no projection still holds a row the fill could give its source and ACL: a row without
150150
* them whose document exists. A row whose document is gone is not the fill's to finish and never
151-
* counts as left. Each read is one index probe while any such row remains.
151+
* counts as left. Each read is one probe of the unfilled-rows index while any such row remains:
152+
* ordered by id and capped at one row so the planner cannot take a sequential scan, which an
153+
* `EXISTS` would leave open by dropping the order and the limit.
152154
*/
153155
async function projectionsFilled(sql: postgres.Sql): Promise<boolean> {
154156
for (const projection of PROJECTION_SOURCE_ACL_TABLES) {
155157
const [row] = await sql.unsafe<Array<{ unfilled: boolean }>>(
156-
`SELECT EXISTS (
157-
SELECT 1 FROM ${projection} s JOIN document d ON d.id = s.document_id WHERE s.acl IS NULL
158-
) AS unfilled`
158+
`SELECT (
159+
SELECT s.id FROM ${projection} s JOIN document d ON d.id = s.document_id WHERE s.acl IS NULL
160+
ORDER BY s.id DESC LIMIT 1
161+
) IS NOT NULL AS unfilled`
159162
)
160163
if (row?.unfilled) return false
161164
}

‎apps/sim/lib/knowledge/search/queries.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2822,6 +2822,24 @@ describe('filters on a resolved scope', () => {
28222822
expect(caps.at(-1)).toBe('20000')
28232823
})
28242824

2825+
it('looks for an unfilled row through the ordered, capped read the partial index serves', async () => {
2826+
traversedRows = [{ id: 'a' }]
2827+
rerankRows = [hit('a', 'src-a')]
2828+
queueTableRows(schemaMock.embedding, rerankRows)
2829+
await handleVectorOnlySearch({
2830+
...params,
2831+
permitted: { kind: 'unbounded', broad: true },
2832+
accessPlan: plan(),
2833+
})
2834+
const probes = statements().filter((query) => query.sql.includes('AS unfilled'))
2835+
expect(probes).toHaveLength(1)
2836+
/** An `EXISTS` drops its order and limit, and the planner then takes a sequential scan. */
2837+
expect(probes[0].sql).not.toContain('EXISTS')
2838+
expect(probes[0].sql.replace(/\s+/g, ' ')).toContain(
2839+
'SELECT ( SELECT ? FROM ? WHERE ? IS NULL ORDER BY ? DESC LIMIT 1 ) IS NOT NULL AS unfilled'
2840+
)
2841+
})
2842+
28252843
it('tests the date through the document inside an on-row walk when the filtered set is unbounded', async () => {
28262844
traversedRows = [{ id: 'a' }]
28272845
rerankRows = [hit('a', 'src-a')]

‎apps/sim/lib/knowledge/search/queries.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,13 @@ const PROJECTION_FILLED_TTL_MS = 60_000
115115

116116
/**
117117
* Whether the ranking projection still holds rows the backfill has not filled. Read off the
118-
* unfilled-rows index in microseconds and remembered briefly: the answer only ever changes once.
118+
* unfilled-rows index in milliseconds and remembered briefly: the answer only ever changes once.
119+
*
120+
* The read asks for the last unfilled row by id, not whether one exists: an `EXISTS` drops its
121+
* order and limit, and while most rows are unfilled the planner expects a sequential scan to
122+
* meet one at once, then walks the whole projection when the unfilled rows sit past the filled
123+
* ones. Ordered by id and capped at one row, the read can only be the partial index, whose
124+
* last entry is the row the fill reaches last.
119125
*/
120126
const projectionFilled = new LRUCache<
121127
ProjectionSourceAclTable,
@@ -134,7 +140,10 @@ const projectionFilled = new LRUCache<
134140
try {
135141
const [row] = await runSearchQuery(context.budget, context.stage, (executor) =>
136142
executor.execute<{ unfilled: boolean }>(sql`
137-
SELECT EXISTS (SELECT 1 FROM ${table} WHERE ${table.acl} IS NULL) AS unfilled`)
143+
SELECT (
144+
SELECT ${table.id} FROM ${table} WHERE ${table.acl} IS NULL
145+
ORDER BY ${table.id} DESC LIMIT 1
146+
) IS NOT NULL AS unfilled`)
138147
)
139148
return !row?.unfilled
140149
} catch {

0 commit comments

Comments
 (0)