Conversation
searchFTS() took a global top-(limit * 10) from the FTS5 index and then dropped the rows outside the requested collection. The over-fetch could not make a post-filter correct, it only moved the cutoff: a collection holding a small share of the index loses every row whenever the global window happens to contain none of its documents, and the caller cannot tell that apart from the collection genuinely having nothing on the subject. This is the keyword-side twin of the vector fix in tobi#847, which replaced the same global-then-filter shape in searchVec() with a collection-scoped scan. searchFTS() kept it, softened only by BM25 being far more selective than ANN. The scope now rides in the FTS5 lookup as a rowid prefilter. documents_fts.rowid is documents.id, so the subquery is a covering-index read on idx_documents_collection and the MATCH still runs against the FTS5 index (EXPLAIN QUERY PLAN: SCAN documents_fts VIRTUAL TABLE INDEX 0:=M3), which is what the CTE exists to protect. The limit * 10 over-fetch is gone: the CTE now takes exactly `limit` rows, because every one of them is in scope. Multi-collection scopes are unaffected. They still fan out per collection and merge by score (tobi#871); each leg is now scoped at the index. Test: 40 short noise documents carrying the term in their titles, plus one long target document in a second collection. Scoped to that second collection the target is the only answer. Before this change the same call returned [], because all 20 candidates in the global window belonged to the noise collection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… prefilter The rowid IN (...) prefilter defeats FTS5 early termination: SQLite drives the query from the collection's whole doclist and probes the FTS5 index once per candidate rowid (EXPLAIN: VIRTUAL TABLE INDEX 0:=M3), about 55ms per in-scope document. On the live index that is 0.02s unscoped versus 21s scoped to a large collection, and 507s for one pass over 19 collections, which on a single-threaded MCP server is a multi-minute outage for every client. Keep the MATCH global and unbounded, MATERIALIZE the complete match set once (corpus-bounded), and apply the collection filter, ORDER BY and LIMIT in the outer query. Correct (the inner set is complete, so the outer filter cannot truncate it) and fast (natural FTS5 operation, EXPLAIN VIRTUAL TABLE INDEX 0:M3). MATERIALIZED is load-bearing: without it the planner may flatten the CTE and refold the collection predicate into the MATCH. Unscoped path unchanged; result rows identical to the prefilter.
Amendment: materialize the match set instead of a rowid prefilterThe scoped path in the first commit was correct but slow. It scoped the FTS5 lookup with a This commit keeps the MATCH global and unbounded,
The On the live index, same match string and identical result rows:
Full per-collection numbers are in the updated description. |
What
searchFTS()scopes a collection filter by materializing the complete FTS5 match set once and filtering it in the outer query, instead of a global top-N post-filter (the original bug) or arowid IN (...)prefilter (correct but slow). This is the keyword-side twin of #847, with the cost characteristic the vector side already has.Why
Two ways to get collection scoping wrong, and this PR avoids both:
Global top-N, then post-filter (the code before this PR).
searchFTS()took a global top-limit * 10from the FTS5 index and dropped the out-of-scope rows afterwards. The over-fetch cannot make a post-filter correct; it only moves the cutoff. A collection holding a small share of the index loses every row whenever the global window contains none of its documents, and the caller cannot tell that apart from the collection genuinely having nothing on the subject: both return[].rowid IN (subquery)prefilter inside the MATCH (the obvious correctness fix). It is correct, but it defeats FTS5 early termination: with an extra rowid constraint SQLite drives the query from the collection's whole doclist and probes the FTS5 index once per candidate rowid.EXPLAIN QUERY PLANshowsSCAN documents_fts VIRTUAL TABLE INDEX 0:=M3(rowid-equality-constrained), which measured about 55ms per in-scope document: 0.02s unscoped versus 21s scoped to a large collection, and 507s for a single pass over 19 collections on the live index (roughly 8,900 documents). On a single-threaded MCP server that turns one scoped auto-expand recall into a multi-minute outage for every client.How
Keep the FTS5 MATCH global and unbounded,
MATERIALIZEthe complete match set once, and apply the collection filter,ORDER BYandLIMITin the outer query:Correct, because the materialized set is complete, so the outer filter cannot truncate it. Fast, because the MATCH is the natural FTS5 operation rather than a per-rowid probe:
EXPLAIN QUERY PLANbecomesMATERIALIZE fts_matches/SCAN documents_fts VIRTUAL TABLE INDEX 0:M3, the same MATCH plan the unscoped path uses. The match set is corpus-bounded (at most one row per matching document).MATERIALIZEDis load-bearing: without it the planner may flatten the CTE and fold the collection predicate back into the MATCH, reintroducing the per-rowid plan.The unscoped path is unchanged: there the MATCH is the whole answer, so it keeps the inner
LIMITand FTS5 early-terminates at the requested count. Multi-collection scopes still fan out per collection and merge by score (#871); each leg is now materialize-and-filter.Measured
Same match string (
the* AND process* AND of*), live index, read-only, result rows identical in every comparison:0:=M3)0:M3)The actual outage recall (a longer auto-expand query) over all 19 collections: 507.7s per fan-out pass before, 2.0s after.
Testing
The scoped
searchFTSbody here is identical to a version validated locally againstbun test test/store.test.ts(green). The regression test in this PR (a scoped collection's only match ranks below the global top-N) fails on the pre-PR post-filter and passes here; it also passes under the prefilter, so it guards correctness but not the cost. A plan-guard (assert the scoped plan isMATERIALIZE/0:M3and not0:=M3) is the right cost guard, but a plan-string assertion is SQLite-version-sensitive, so it is called out here rather than baked in.