Skip to content

Make full-text matches predicate null-rejecting so it uses the markdown GIN index - #5680

Merged
lukemelia merged 2 commits into
mainfrom
cs-12408-full-text-federated-search-bypasses-its-gin-index-and-scans
Aug 4, 2026
Merged

Make full-text matches predicate null-rejecting so it uses the markdown GIN index#5680
lukemelia merged 2 commits into
mainfrom
cs-12408-full-text-federated-search-bypasses-its-gin-index-and-scans

Conversation

@lukemelia

Copy link
Copy Markdown
Contributor

Problem

Full-text matches searches are far slower than equivalent non-full-text searches, and a guaranteed zero-result term costs the same as a common one — the server rebuilds and scans every row's text vector instead of using the GIN index.

IndexQueryEngine.matchesCondition() emitted:

to_tsvector('english', markdown_search_text(ph.markdown))
  @@ websearch_to_tsquery('english', $query)

against prerendered_html (ph), which is attached through a LEFT JOIN. Because markdown_search_text() coalesces null markdown to '', Postgres cannot prove the predicate null-rejecting and cannot reduce the outer join. It drives the query from boxel_index, joins the rendered rows, and computes each tsvector at query time — bypassing prerendered_html_markdown_fts_idx.

Fix

Prepend an explicit null-rejecting guard to the Postgres predicate:

ph.markdown IS NOT NULL AND to_tsvector('english', markdown_search_text(ph.markdown))
  @@ websearch_to_tsquery('english', $query)

This is semantically equivalent — the sanitizer produces an empty tsvector for absent markdown, which never matches a non-empty query, so those rows were already excluded — but it lets the planner reduce the join and reach the GIN index. It stays correct when the leaf is nested under not / any (every([predicate]) wraps the compound in parens, so precedence holds).

The same code path drives both prerendered_html and prerendered_html_working. The SQLite path (a coalesce + LIKE) is unchanged — it has no equivalent planner restriction.

Tests

The prior planner test EXPLAIN'd a bare prerendered_html query with no join, so it passed even with the bug present. Reworked it to EXPLAIN the real join shape and assert the guard reduces the outer join to an inner join, while the unguarded (pre-fix) predicate stays a left join. Join type is the size-independent signal: index selection is dataset-dependent (the small seed prefers the realm B-tree + filter; the GIN index wins at scale), but the join reduction is exactly what makes the index reachable.

Existing coverage for top-level matches, matches inside every / any / not, and rows without a rendered row continues to pass (26/26).

Follow-ups (out of scope)

The federated-search fan-out amplification called out in the ticket — one search per realm, a duplicate exact-count query per realm, and per-realm pagination — is tracked as separate follow-up work; this change is the planner fix only.

🤖 Generated with Claude Code

…wn GIN index

The `matches` full-text predicate ran against `prerendered_html` (`ph`),
attached via a LEFT JOIN. Because `markdown_search_text()` coalesces null
markdown to '', Postgres could not prove the predicate null-rejecting,
could not reduce the outer join, and recomputed a tsvector for every
joined row at query time instead of using `prerendered_html_markdown_fts_idx`.
A guaranteed-zero-result term cost the same as a common term.

Prepend `ph.markdown IS NOT NULL AND` to the Postgres predicate. It is
semantically equivalent (an empty tsvector never matches a non-empty
query, so absent-markdown rows were already excluded) but lets the planner
reduce the join and reach the GIN index. The same code drives both
`prerendered_html` and `prerendered_html_working`. The SQLite path is
unchanged.

Rework the planner test to EXPLAIN the real join shape and assert the
guard reduces the outer join to an inner join (the size-independent
mechanism that unlocks the index at scale), while the unguarded predicate
stays a left join.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files      1 suites   2h 53m 24s ⏱️
3 809 tests 3 795 ✅ 14 💤 0 ❌
3 828 runs  3 814 ✅ 14 💤 0 ❌

Results for commit 508d1bb.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   14m 8s ⏱️ + 2m 11s
2 033 tests +1  2 033 ✅ +1  0 💤 ±0  0 ❌ ±0 
2 112 runs  +1  2 112 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 508d1bb. ± Comparison against earlier commit 25daefb.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts how the runtime-common index query engine emits Postgres full-text matches predicates so Postgres can treat them as null-rejecting and reduce the prerendered_html outer join, making the markdown GIN index reachable for full-text queries.

Changes:

  • Add an explicit ph.markdown IS NOT NULL guard to the Postgres matches predicate in IndexQueryEngine to enable outer-join reduction and index usage.
  • Update the realm-server integration test to EXPLAIN a join-shaped query and assert join reduction via the JSON plan’s "Join Type".

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/runtime-common/index-query-engine.ts Adds a null-rejecting guard to the Postgres matches predicate to enable join reduction and reach the markdown GIN index.
packages/realm-server/tests/matches-filter-integration-test.ts Reworks the planner test to assert join reduction (inner vs left join) for guarded vs unguarded predicates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/realm-server/tests/matches-filter-integration-test.ts Outdated
Comment thread packages/realm-server/tests/matches-filter-integration-test.ts Outdated
Address review on the matches-filter integration test:

- The EXPLAIN join-shape test composes the predicate itself, so it proves
  the guard *would* reduce the join but can't catch IndexQueryEngine
  dropping it. Add a test that captures the SQL the engine actually emits
  for a `matches` search and asserts the `ph.markdown IS NOT NULL` guard
  is present, so an engine regression fails.
- Bind the realm URL and query term as parameters in the EXPLAIN query
  instead of interpolating them, so the SQL stays quoting-safe.
- Drop the temporal "pre-fix" phrasing from the join-shape comment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lukemelia
lukemelia marked this pull request as ready for review August 4, 2026 15:55
@lukemelia
lukemelia requested review from a team and jurgenwerk August 4, 2026 15:55
@lukemelia
lukemelia merged commit c7f44e8 into main Aug 4, 2026
90 of 94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants