Repository navigation
perf(core): run FTS text conditions only on the GIN candidates - #1794
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
phernandez
added this pull request to stack #1797
October 20, 2026 15:56
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
force-pushed
the
search-fts-candidate-fence
branch
from
October 20, 2026 16:07
1bfa9a5 to
fdf6cb5
Compare
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2 of #1791, stacked on #1793: FTS predicate placement.
Problem
compile_fts_filterjoinssearch_indexto 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 oneEXISTSprobe intosearch_index_fts_chunksper 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 0subquery aliasedsearch_index. Postgres cannot push conditions through that fence, so the text conditions (word, script) run above it, on candidates only. Every other reference tosearch_indexis 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: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_candidatesrunsEXPLAIN (FORMAT JSON)on the compiled search SQL against Postgres. It asserts that the document-vector condition (to_tsvector) appears only on aSubquery Scan, never on a scan ofsearch_index. Postgres always pushes a condition on a plain table into that table's scan, so the plan shape is deterministic.{'Index Scan'} == {'Subquery Scan'}. With the fence, it passes.pymilvuserrors.🤖 Generated with Claude Code
https://claude.ai/code/session_016N9c6ouiT4Wid3arGR41Dv