Faceted-search filter pushdown for BM25 scans - #408
Closed
tjgreen42 wants to merge 17 commits into
Closed
Conversation
For queries combining a scalar filter with a BM25 ORDER BY ... LIMIT, push the filter into Block-Max WAND retrieval so the top-k heap only considers matching documents, instead of ranking everything and discarding non-matches in a Filter node afterward. The optimization is always exact: PostgreSQL keeps its Filter node above the index scan, so the pushed-down allow-list need only be a superset of matching rows. Gated by pg_textsearch.enable_facet_pushdown (default on) and pg_textsearch.facet_selectivity_threshold (default 0.12). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Codecov upload step gated the whole build on an external service: a
Codecov CLI GPG self-verification failure ("Could not verify signature")
fails the build even though coverage was captured and the separate Coverage
gate (85% minimum + no-regression check) passed. Make the upload best-effort.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
tjgreen42
marked this pull request as ready for review
June 9, 2026 01:34
Resolve Makefile REGRESS conflict by keeping both new regression tests: 'facet' (this branch) and 'segment_reclaim' (from #406).
The faceted-search filter pushdown built its allow-list from a heap scan, which yields each row's live tuple and therefore the CTID at the tip of any HOT update chain. The BM25 segment, like every Postgres index, stores the CTID of the HOT-chain root (that is the TID the index build callback receives). After even a single HOT update the two diverge, so the allow-list no longer matched the segment's stored CTIDs and BMW silently excluded matching documents from the top-k. This is not caught by the Filter node above the index scan: the omission is a false negative inside the index (a matching row never returned), not a false positive the recheck could drop. On an UPDATE-heavy load ~12% of multi-term faceted queries returned incorrect results. Remap each collected TID to its HOT-chain root with heap_get_root_tuples() before sorting the allow-list, so membership tests compare like-for-like. HOT chains never span pages, so the remap stays within each block; for un-updated rows the root is the tuple itself and the remap is a no-op. Add a regression test that HOT-updates a non-indexed column (fillfactor=50 so the update stays on-page) and asserts the pushdown still returns the updated matching row, identical to the post-filter path.
# Conflicts: # src/access/am.h
The faceted-search filter pushdown built its allow-list with a full heap scan (O(table)), evaluating the operator on every row. At 1M rows that ~100-200ms per-query build dominated the query and made the pushdown slower than the post-filter baseline it was meant to beat. Build the allow-list from a suitable index on the facet column: a valid, non-partial index whose leading key is the facet column and whose opclass supports the operator with a matching collation, scanned via index_getnext_tid (O(matching rows)). Index entries reference HOT-chain roots, as the BM25 segment does, so this path needs no HOT-root remap. A full heap scan is the fallback when no suitable index exists. index_getnext_tid yields a safe superset (no heap visibility check); the Filter node above the scan is the exact recheck. At 1M MS-MARCO rows the forced-pushdown latency drops from 114-206ms to 7-80ms, and at a 1% facet the pushdown beats the post-filter path (7.3ms vs 11.6ms). Add pg_textsearch.log_facet to report the build path taken, plus regression coverage for the index path, the heap fallback, and HOT-update correctness with a facet index present. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
A parameter sweep on MS-MARCO locates the pushdown/baseline crossover at ~1.9% facet selectivity on 1M rows: below it the index-backed allow-list beats the post-filter limit-doubling path (up to 2.2x at 0.2%), above it the baseline wins (up to 1.9x at 5%). The previous default of 0.12 engaged the pushdown well into the regime where it loses. The crossover shifts with corpus size (~3% at 250k, ~1.9% at 1M, lower beyond), so 0.02 is a slightly conservative default that captures the strong low-selectivity wins across scales. The GUC remains tunable. The queries and strings regression outputs change by one debug scan NOTICE line each: their category= facets now sit above the lower gate, so the pushdown disengages there. Result rows are unchanged. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
Three fixes to the index-backed facet allow-list, each guarding against silently wrong results: 1. Cross-type operator subtype. The btree/hash scan key was built with an InvalidOid subtype, which means "same type as the index column". For a cross-type facet operator (e.g. float4 = float8, a hash-opfamily member) a hash index then probes with the wrong hash function and returns an empty allow-list, dropping every matching row. btree masks this via its operator recheck, but hash does not. Pass the operator's right-hand type as the subtype, as ExecIndexBuildScanKeys does. 2. Broken HOT chains (indcheckxmin). The index selection re-implements the planner's usability filter but omitted the indcheckxmin guard, so under an old snapshot a facet index built over broken HOT chains could miss visible matching rows. Reject such indexes (mirroring plancat.c) and fall back to the heap scan. 3. Row-Level Security. The allow-list is built by scanning the base table directly, with no RLS awareness, so a pushed-down predicate was evaluated over rows the caller cannot see and their count was reported via log_facet. Skip the pushdown when the clause carries a security level or the relation has RLS/security-barrier quals, leaving those queries to the post-filter path. Add regression coverage: a cross-type facet via a hash index (must return the matching rows, not an empty set) and an RLS table (the pushdown must disengage and return only visible rows). Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
…eak) The planner stashes a facet spec in a per-backend channel while costing a BM25 index path; the scan consumes it at execution. But costing a path does not mean executing it: when the BM25 path is costed yet a seqscan wins (or the statement is only EXPLAINed), the spec is stashed and never consumed, and it was cleared only at transaction end. A later statement on the same BM25 index in the same transaction then consumed the stale spec and was silently filtered to the earlier query's facet -- e.g. a plain ORDER BY body <@> q LIMIT k returning only the rows that matched a previous query's WHERE cat = 'x'. Bound the spec's lifetime to a single statement: - Reset the pending spec at the start of tp_try_store_facet, so re-planning any BM25 path (including the later statement's) drops a stale spec. - Add an ExecutorEnd hook that drops the pending spec after each statement, covering the executed-but-not-consumed case and cached generic plans that skip re-planning. Add a regression test reproducing the leak: a faceted query forced onto a seqscan stashes a spec, and a forced-BM25 non-faceted query in the same transaction must still return all rows. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
Update the README facet pushdown section: the allow-list is built with an index scan on the facet column (heap scan fallback), the facet_selectivity_threshold default is 0.02, and add pg_textsearch.log_facet. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
Benchmarks the facet pushdown on MS-MARCO (WHERE facet_id < N ORDER BY passage_text <@> q LIMIT 10) across a facet-selectivity sweep, and optionally compares against ParadeDB pg_search on identical data and queries. The pg_textsearch load is scale-parametrizable via MAXROWS and FACET_BUCKETS; the query sweep forces the BM25-scan plan to isolate the pushdown against the post-filter baseline and asserts on-vs-off parity. Results and usage in benchmarks/facet/README.md. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
Keep the GUC reference complete: enable_facet_pushdown, facet_selectivity_threshold, and log_facet. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
The sweep disabled statement_timeout per cell via set_config, which did not reliably protect a cell: at very low selectivity the pre-PR path re-drives the BM25 scan with a growing internal limit, re-reading a common term's large posting lists each pass, and one query on the full 138M-passage v2 corpus runs long enough to abort the whole run. Cap each query at cap_ms (default 60s, -v cap_ms=...), catch the cancellation, and censor the query (count it and record it at the cap) instead of failing. Report the censored count per cell. Apply the same guard to the parity check (skip a capped pair). Also drop the function before recreating it so a re-run against an existing database does not fail on the changed return type, and set on_default to the shipped 0.02 gate. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
Self-review of the facet pushdown surfaced two correctness bugs and one visibility bug, all introduced by this feature. Wrong results when a facet spec binds to the wrong same-index scan (high). The pending spec is a per-backend channel consumed by the AM keyed only on the BM25 index OID, and the allow-list is a hard exclusion in Block-Max WAND. When one statement has two BM25 scans on the same index (e.g. two subqueries), or a spec outlives its statement via a concurrently open cursor, a scan could consume a spec built for a different scan and silently drop all of its rows -- the executor's Filter cannot re-add rows the scan never produced. Add an ExecutorRun hook that sanitizes the pending spec against the plan being executed: keep it only when exactly one IndexScan on the spec's index carries the spec's clause, otherwise clear it so those scans use the correct post-filter path. Reproduced by the new two-scan and concurrent-cursor tests. Leaked facet constant (medium). The by-reference clause constant copied into TopMemoryContext was never freed after the spec was consumed, because the free helper was gated on is_valid, which consumption had already cleared. Free on the pointer itself so the ExecutorEnd reset reclaims it. Heap-fallback snapshot (medium). The no-index allow-list was built under GetActiveSnapshot() rather than the scan's own snapshot, so under a cursor or a multi-snapshot context it could drop a row the scan can see. Thread the scan's xs_snapshot through. EXPLAIN observability. On PostgreSQL 18+ the BM25 index scan now reports facet pushdown engagement via the per-node EXPLAIN hook: a "BM25 Facet Pushdown:" line showing whether the allow-list was built via the facet index or a heap scan, and its size. The line is present only under EXPLAIN ANALYZE when the pushdown engaged, so it distinguishes a facet-index-backed scan from a plain BM25 scan whose facet is left to the post-filter. PostgreSQL 17 has no per-node EXPLAIN hook; log_facet reports the same there. Tests. facet.sql gains coverage that exercises the index across multiple on-disk segments and the in-memory memtable in one scan, plus the two-scan and cursor correctness cases. New facet_explain.sql covers the EXPLAIN annotation (with a PG17 alternative expected, since the line is PG18-only). Also drop the unrelated Codecov ci.yml change from this branch. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
Adds facet_bugs.sql covering the remaining review findings that lacked
direct coverage:
- F2 (leaked facet constant): route the pending spec's by-reference value
through a dedicated named memory context ("pg_textsearch facet spec") so its
lifetime is observable in pg_backend_memory_contexts. The test runs 2000
faceted queries with distinct text constants and asserts the context stays
bounded; with the pre-fix is_valid-gated free it grows to ~500 KB and the
test fails, so it is a genuine regression guard.
- F3 (heap-fallback snapshot): exercise the no-facet-index (heap fallback)
allow-list through a cursor and assert parity with the post-filter path.
(A test that fails only on the pre-fix GetActiveSnapshot() is not
constructible: during cursor execution PostgreSQL pushes the portal snapshot
as the active one, so it equals the scan snapshot; the fix is defensive.)
- NULL facet values: a strict operator excludes NULLs; assert parity on both
the index and heap build paths.
- Rescan / re-execution: the spec is one-shot per statement, so assert a
re-executed faceted scan returns the correct rows every time.
Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
A facet index built over broken HOT chains carries indcheckxmin, and must be skipped for the allow-list when its xmin is not yet old enough under the query snapshot (mirroring plancat.c) -- otherwise the heap scan is used. Build the BM25 index first so it stays usable, then in one transaction HOT-update the facet column and create the facet index; the same-transaction faceted query reports "heap scan" (log_facet), and the test confirms the facet index really carried indcheckxmin. Removing the guard would make it read the broken index instead, changing the reported path. Copilot-Session: 5c8dcfe2-5937-4985-8735-27f57f418553
tjgreen42
added a commit
that referenced
this pull request
Aug 11, 2026
## Summary A filtered BM25 top-k query — `WHERE <filter> ORDER BY body <@> q LIMIT k` — is planned as a BM25 top-k index scan with `<filter>` applied as an executor Filter above it. When the scan's top-k by score contains few Filter-matching rows, the executor re-drives the scan with a geometrically growing internal limit, re-scoring from scratch each round (about `log2(1/s)` rounds for a filter of selectivity `s`). This seeds the scan's internal top-K from the planner's estimated filter selectivity — `seed_K = ceil(margin * k / s)`, capped at `TP_MAX_QUERY_LIMIT` — so a single scoring pass usually surfaces enough matching rows. The executor Filter and the existing backoff still determine the result, so output is identical to the unseeded plan regardless of estimate accuracy. The change is confined to `tp_costestimate`; the scan, BMW, and on-disk formats are unchanged. Filtering during scoring instead (an allow-list keyed by heap ctid) would require a `doc_id`→`ctid` translation per scoring candidate, since scoring runs in `doc_id` space; that is slower than seeding. ## GUCs | GUC | Type | Default | Description | |-----|------|---------|-------------| | `pg_textsearch.filtered_seed` | bool | on | Enable selectivity seeding of the internal top-K. | | `pg_textsearch.filtered_seed_margin` | real | 3.0 | `seed = ceil(margin * LIMIT / selectivity)`; range [1, 1000]. | ## Benchmark MS-MARCO v2 (`msmarco_facet`, 138,364,158 rows), PostgreSQL 17.9, optimized (`-O2`) builds, top-10, `facet_id < N` filter, 50 warm queries, single backend. `filtered_seed` off (prior behavior) vs on: | selectivity | off (avg / p50 ms) | on (avg / p50 ms) | |-------------|-------------------:|------------------:| | 0.1% | 1076 / 631 | 202 / 153 | | 1% | 627 / 316 | 109 / 66 | | 5% | 408 / 224 | 87 / 49 | Result sets are identical off/on (parity test included); no timeouts. For reference, ParadeDB (pg_search 0.16.1) on the same server, table, and queries — `facet_id` as a fast field, English stemming and stopwords matched to `text_config=english` — runs the same workload in 601 / 654 / 708 ms (avg) at 0.1 / 1 / 5%. Cross-engine tokenization differs, so result sets are comparable, not identical. ## Testing New `filtered_seed` regression test: plan assertion, seed on/off parity, an independent regex oracle, no-filter no-op, margin robustness, and GUC range checks. Full regression passes on PostgreSQL 17 and 18. Known limitation tracked in #435. Supersedes #408.
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
1 participant
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.
Pushes a scalar
col OP constrestriction into Block-Max WAND retrieval for queries that pair it with a BM25ORDER BY ... LIMIT, so the top-k heap considers only documents matching the facet. Exact by construction: PostgreSQL keeps its Filter node above the index scan as the recheck, so the pushed-down allow-list only needs to be a superset of the matching rows.How the allow-list is built
When a suitable index exists on the facet column, the allow-list is built with an index scan on it — O(matching rows). A suitable index is valid, non-partial, has
amgettuple, has the facet column as its leading key, and its opclass supports the operator with a matching collation.index_getnext_tidyields a superset (no heap visibility check), which the Filter recheck bounds. Index entries reference HOT-chain roots, matching the CTIDs the BM25 segment stores. A full heap scan is the fallback when no suitable index exists.pg_textsearch.log_facetreports which path (index vs heap) built the allow-list.Correctness
float4 = float8): the scan-key subtype is the operator's right-hand type, so hash and btree indexes use the operator's cross-type support proc.indcheckxminis still suspect under the query snapshot is skipped (as the planner does inplancat.c) and the heap scan is used.ExecutorRunhook sanitizes it against the running plan — it is kept only when exactly one BM25 scan on that index carries the clause. Two BM25 scans on one index in a single statement, or a spec outliving its statement via an open cursor, fall to the post-filter path instead of binding to the wrong scan.Configuration
pg_textsearch.enable_facet_pushdownpg_textsearch.facet_selectivity_thresholdANALYZE).pg_textsearch.log_facetOn PostgreSQL 18+,
EXPLAIN (ANALYZE)shows aBM25 Facet Pushdown:line on the BM25 index scan when the pushdown engaged (via index "..."/via heap scan, with the allow-list size); its absence means the facet was left to the post-filter. PostgreSQL 17 has no per-nodeEXPLAINhook, solog_facetreports the same there.Benchmark
Full MS-MARCO v2 (138,364,198 passages), PostgreSQL 17.9 and the extension both built
-O2with assertions off, on a tuned instance (shared_buffers32GB). Uniform synthetic facet over 1000 buckets,WHERE facet_id < N ORDER BY passage_text <@> q LIMIT 10, 200 queries/cell, BM25-scan plan forced (enable_bitmapscan=off). BM25 index 17GB (1121s, 12 parallel workers); facet btree 915MB. The 83GB working set exceedsshared_buffers, so the run is partly I/O-bound. Latency per query:The pushdown cost scales with the number of matching rows (0.1% is a 138K-TID allow-list, 5% is 6.9M), so it wins on selective facets and loses on broad ones. The crossover is between 1% and 2.5%; the
0.02default threshold sits at the crossover, so with the default gate the pushdown engages below 2% selectivity and the baseline path runs at and above it. Top-10 results are identical with the pushdown on and off (199/199 queries, 0 mismatches).At 0.1% selectivity one common-term query does not finish within 60s on the baseline: its posting lists are evicted under memory pressure and the limit-doubling re-drive re-reads them each iteration. The pushdown answers it in ~0.3s. That query is censored at 60s in the baseline 0.1% average.
At extreme selectivity the planner can also choose a plain index scan on the facet column plus a top-N sort. The benchmark harness (load, query sweep, and the ParadeDB comparison) is in
benchmarks/facet/.Tests
Regression coverage for the index and heap build paths, engagement across multiple on-disk segments and the in-memory memtable in one scan, HOT-update correctness, cross-type operators via a hash index, RLS disengagement, two-scan and concurrent-cursor scan isolation, and the PostgreSQL 18 EXPLAIN annotation (with a PG17 alternative expected). All SQL regression tests pass on PostgreSQL 17 and 18.