diff --git a/packages/realm-server/tests/matches-filter-integration-test.ts b/packages/realm-server/tests/matches-filter-integration-test.ts index a0bb946efa9..ddd0a1c3ff4 100644 --- a/packages/realm-server/tests/matches-filter-integration-test.ts +++ b/packages/realm-server/tests/matches-filter-integration-test.ts @@ -357,33 +357,105 @@ module(basename(import.meta.filename), function () { ); }); - test('planner uses prerendered_html_markdown_fts_idx for matches queries', async function (assert) { - // The filter emits `to_tsvector('english', markdown_search_text(ph.markdown))` - // which must match the GIN index expression exactly. With only a handful - // of seeded rows PG will ordinarily prefer a seqscan; disabling seqscan - // forces the planner to reveal whether the GIN index is a viable - // candidate at all — which is the property we actually care about. + test('IndexQueryEngine emits the null-rejecting guard in its matches SQL', async function (assert) { + // The join-shape test below proves the guard *would* reduce the join, but + // it composes the predicate itself, so it can't catch IndexQueryEngine + // dropping the guard. This test closes that gap: it captures the SQL the + // engine actually emits for a `matches` search and asserts the guard is + // present, so a regression in the engine fails here rather than passing on + // a predicate the engine no longer produces. + let executed: string[] = []; + let originalExecute = dbAdapter.execute.bind(dbAdapter); + dbAdapter.execute = (( + sql: string, + opts?: Parameters[1], + ) => { + executed.push(sql); + return originalExecute(sql, opts); + }) as typeof dbAdapter.execute; + try { + await engine.searchCards(new URL(testRealmURL), { + filter: { matches: 'mango' }, + }); + } finally { + dbAdapter.execute = originalExecute; + } + + let guard = `ph.markdown IS NOT NULL AND to_tsvector('english', markdown_search_text(ph.markdown))`; + let matchesSql = executed.filter((sql) => + sql.includes('markdown_search_text(ph.markdown)'), + ); + assert.ok( + matchesSql.length > 0, + 'the engine ran a full-text matches query', + ); + for (let sql of matchesSql) { + assert.ok( + sql.includes(guard), + `emitted matches SQL should include the null-rejecting guard; got: ${sql}`, + ); + } + }); + + test('the null-rejecting guard reduces the prerendered_html LEFT JOIN to an inner join', async function (assert) { + // The engine attaches prerendered_html to boxel_index through a LEFT JOIN + // (see `prerenderedJoin`) and runs the `matches` predicate against the + // joined `ph`. `markdown_search_text` coalesces null markdown to '', which + // hides the predicate's null-rejection from the planner: without the + // `ph.markdown IS NOT NULL` guard the planner cannot reduce the outer join, + // so it drives the query from boxel_index and recomputes `to_tsvector` for + // every joined row instead of reaching `prerendered_html_markdown_fts_idx`. + // The guard restores the join reduction — and that reduction is exactly + // what makes the markdown GIN index reachable at scale (on a realm with + // thousands of rows the planner then BitmapAnds the realm B-tree with the + // markdown GIN; with only a handful of seeded rows here it prefers the + // realm B-tree plus a filter, so asserting the index name would be + // dataset-dependent). The join type is the size-independent signal, so we + // assert directly on it: the guarded predicate yields an inner join and the + // unguarded predicate stays a left join. A build that drops the guard fails + // here rather than silently reintroducing the slowdown. // - // SET LOCAL is bound to a transaction, so we run SET/EXPLAIN/COMMIT on - // the same pooled connection via withConnection. Each inner query is - // its own round-trip, so EXPLAIN's result is returned cleanly. - let plan = await dbAdapter.withConnection(async (run) => { - await run(['BEGIN']); - await run(['SET LOCAL enable_seqscan = OFF']); - let rows = await run([ - `EXPLAIN (FORMAT JSON) - SELECT url FROM prerendered_html - WHERE to_tsvector('english', markdown_search_text(markdown)) - @@ websearch_to_tsquery('english', 'mango')`, - ]); - await run(['COMMIT']); - return rows; - }); + // EXPLAIN's JSON output stringifies each join's planner-chosen strategy as + // `"Join Type":"Inner"` / `"Join Type":"Left"`; this query has exactly one + // join, so a substring check is unambiguous. + // + // The realm URL and query term are bound as parameters ($1/$2) rather than + // interpolated, so the SQL stays quoting-safe whatever their contents; only + // the predicate *shape* — which is test-controlled, not data — is composed + // inline. + let tsPredicate = `to_tsvector('english', markdown_search_text(ph.markdown)) @@ websearch_to_tsquery('english', $2)`; + let joinShapeExplain = (predicate: string) => + `EXPLAIN (FORMAT JSON) + SELECT i.url + FROM boxel_index AS i + LEFT JOIN prerendered_html AS ph + ON ph.url = i.url AND ph.realm_url = i.realm_url AND ph.type = i.type + WHERE i.realm_url = $1 + AND ${predicate}`; + + let explainJoinShape = async (predicate: string) => + JSON.stringify( + await dbAdapter.execute(joinShapeExplain(predicate), { + bind: [testRealmURL, 'mango'], + }), + ); + + let guarded = await explainJoinShape( + `ph.markdown IS NOT NULL AND ${tsPredicate}`, + ); + let unguarded = await explainJoinShape(tsPredicate); - let planText = JSON.stringify(plan); assert.ok( - planText.includes('prerendered_html_markdown_fts_idx'), - `plan should reference the GIN index; got: ${planText}`, + guarded.includes('"Join Type":"Inner"'), + `guarded predicate should reduce the outer join to an inner join; got: ${guarded}`, + ); + assert.notOk( + guarded.includes('"Join Type":"Left"'), + `guarded predicate should leave no unreduced outer join; got: ${guarded}`, + ); + assert.ok( + unguarded.includes('"Join Type":"Left"'), + `unguarded predicate should leave the outer join unreduced — the bug the guard fixes; got: ${unguarded}`, ); }); }); diff --git a/packages/runtime-common/index-query-engine.ts b/packages/runtime-common/index-query-engine.ts index 9795b2a478a..04a2e4b3a94 100644 --- a/packages/runtime-common/index-query-engine.ts +++ b/packages/runtime-common/index-query-engine.ts @@ -1293,7 +1293,18 @@ export class IndexQueryEngine { : [ dbExpression({ pg: [ - `to_tsvector('english', markdown_search_text(ph.markdown)) @@ websearch_to_tsquery('english',`, + // The leading `ph.markdown IS NOT NULL` is a null-rejecting + // guard, not a behavior change: `markdown_search_text` coalesces + // null markdown to '' and a non-empty query never matches an + // empty tsvector, so absent-markdown rows are already excluded. + // But because that coalesce hides the null-rejection from the + // planner, without this guard it cannot reduce the LEFT JOIN to + // prerendered_html, drives the query from boxel_index, and + // recomputes every row's tsvector at query time instead of using + // the markdown GIN index. The explicit guard restores the + // reduction so the index is used, and stays correct under `not` + // and `any` (an absent row still can't match a positive term). + `ph.markdown IS NOT NULL AND to_tsvector('english', markdown_search_text(ph.markdown)) @@ websearch_to_tsquery('english',`, param(filter.matches), `)`, ],