Skip to content

perf(core): run FTS text conditions only on the GIN candidates - #1794

Merged
phernandez merged 1 commit into
search-read-costfrom
search-fts-candidate-fence
Oct 20, 2026
Merged

phernandez merged 1 commit into
search-read-costfrom
search-fts-candidate-fence

Conversation

@phernandez

Copy link
Copy Markdown
Member

Part 2 of #1791, stacked on #1793: FTS predicate placement.

Problem

compile_fts_filter joins search_index to a UNION of GIN candidates: parent matches plus chunk matches. The planner estimates one row from that UNION, so it pushes the document-vector condition down onto the scan of every row in scope and joins the candidates afterwards. That condition runs one EXISTS probe into search_index_fts_chunks per query operand, for every scoped row, before the join.

Change

The candidate join and the cheap shared conditions (scope, type, candidate keys) now sit in an OFFSET 0 subquery aliased search_index. Postgres cannot push conditions through that fence, so the text conditions (word, script) run above it, on candidates only. Every other reference to search_index is unchanged. The metadata join and the title and permalink conditions still apply outside. Queries with no candidate join (no text, or stopword-only text) are unchanged.

Measured on the production tenant from #1791

4 projects, entity filter, "meeting budget review", search page limit 130, EXPLAIN (ANALYZE, BUFFERS), read-only:

buffer touches chunk probes (operand subplan loops)
before 1,137k 19,221
after 516k 7,992

Timings varied with the compute's cache state: the same plan ran from 1.8 s warm to 28.5 s cold. Buffers are the comparable number.

About 259k of the remaining touches are the chunk-candidate sequential scan, which detoasts every chunk's tsvector. That is the next PR in the stack. It needs a way to make the planner take the GIN index, because it does not cost TOAST.

Tests

  • test_postgres_text_conditions_run_only_on_candidates runs EXPLAIN (FORMAT JSON) on the compiled search SQL against Postgres. It asserts that the document-vector condition (to_tsvector) appears only on a Subquery Scan, never on a scan of search_index. Postgres always pushes a condition on a plain table into that table's scan, so the plan shape is deterministic.
  • Load-bearing: without the fence, the test fails with {'Index Scan'} == {'Subquery Scan'}. With the fence, it passes.
  • 434 repository search tests pass on Postgres. Lint is clean; typecheck shows only the pre-existing pymilvus errors.

🤖 Generated with Claude Code

https://claude.ai/code/session_016N9c6ouiT4Wid3arGR41Dv

@phernandez phernandez added the cost-review-2026-10 October 2026 COGS review: cost leaks, heavy tenants, and production error triage label Oct 20, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T16:11:07.489954Z fdf6cb5 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

The planner estimated one row from the candidate UNION, so it pushed the
document-vector condition, a per-operand EXISTS into the chunk table, down
onto the scan of every row in scope and joined the candidates afterwards.
On an 8k-note project that was about 19k chunk probes and 1.1M buffer
touches per query. An OFFSET 0 subquery now holds the candidate join plus
the cheap scope, type and key filters, so the text conditions run above it,
on the candidates only: 8k probes and 516k buffer touches for the same query.

Refs #1791

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016N9c6ouiT4Wid3arGR41Dv
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez
phernandez force-pushed the search-fts-candidate-fence branch from 1bfa9a5 to fdf6cb5 Compare October 20, 2026 16:07
@phernandez
phernandez merged commit b096570 into main Oct 20, 2026
36 checks passed
@phernandez
phernandez deleted the search-fts-candidate-fence branch October 20, 2026 16:37
phernandez added a commit that referenced this pull request Oct 20, 2026
Part 3 of #1791, stacked on #1794: the chunk-candidate leg.

## Problem
The FTS candidate UNION reads `search_index_fts_chunks` for chunks
matching the query's operands. The chunk `tsvector`s are stored in
TOAST, and the planner does not cost TOAST. It priced a sequential scan
at the size of the heap: 81 MB on the #1791 tenant, against 375 MB of
TOAST. So it chose the sequential scan over
`idx_search_index_fts_chunks_fts`, then detoasted every chunk to test
it.

After #1794 this leg is about half of the remaining buffer touches (259k
of 516k on the measured query). Production: 19.5 s cold, 0.43 s warm.

## Change
FTS statements that join text candidates
(`CompiledFilter.prefer_index_scans`) run with `enable_seqscan` off.
Statements without a text join, such as filtered listings, plan
normally.

The setting is local to the statement:
- Inside a savepoint (a caller's session, or the relaxed-retry path),
the previous value is restored before the savepoint is released. A
rolled-back savepoint (tsquery syntax error) reverts it on its own.
- Without a savepoint the session is FTS's own, so the setting ends with
its transaction.

`enable_seqscan = off` does not forbid sequential scans. It prices them
out, so a table with no usable index still plans.

## Evidence
On a production-shaped local table (95k chunks, 939 MB TOAST, 7 MB heap,
GIN index):

| query | default plan | buffers | GIN | buffers |
|---|---|---|---|---|
| 26% of chunks match | GIN (planner picks it here) | 909 | GIN | 909 |
| 99% of chunks match | seq scan, 1,537 ms | 481k | bitmap, 17 ms | 921
|

The GIN bitmap is exact, so it never rechecks and never detoasts. In
production the planner estimated 16% of chunks matching and chose the
sequential scan.

I could not measure the whole statement in production: the read-only
connection rejects `SET LOCAL`. To confirm before or after deploy, run
this in the Neon SQL editor on a large tenant, with the rendered FTS SQL
from #1794:

```sql
BEGIN READ ONLY;
SET LOCAL enable_seqscan = off;
EXPLAIN (ANALYZE, BUFFERS) <fenced FTS statement>;
ROLLBACK;
```

The `Seq Scan on search_index_fts_chunks candidate_chunk` node (about
259k buffers) should become a Bitmap Heap Scan.

## Tests
- `test_postgres_chunk_candidates_use_the_gin_index`: the compiled
search statement, run through `execute_fts_statement`, plans no `Seq
Scan` on `search_index_fts_chunks`.
- `test_postgres_fts_leaves_the_callers_session_planning_normally`:
after a search on a caller's session, `enable_seqscan` is `on` again.
Covered for a normal query and for a malformed one that takes the
relaxed retry.
- **Load-bearing:** ignoring the flag fails the GIN test (`'Seq Scan'
not in {'Index Scan', 'Seq Scan'}`). Dropping the restore fails both
session cases. Restored, all pass.
- 618 repository and service search tests pass on Postgres. Lint is
clean; typecheck shows only the pre-existing `pymilvus` errors. SQLite
is unaffected (the flag defaults to false).

## Still open in #1791
- `search.count` repeats the FTS cost. #1794's fence and this PR both
apply to it, since it uses the same compiled filter. Capping it or
turning it into a has-more probe changes the API's `total`, which is a
product call.
- The HNSW manifest join (covering index) is a separate, smaller change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_016N9c6ouiT4Wid3arGR41Dv

Signed-off-by: phernandez <paul@basicmachines.co>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cost-review-2026-10 October 2026 COGS review: cost leaks, heavy tenants, and production error triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant