Conversation
github-actions
Bot
force-pushed
the
master
branch
5 times, most recently
from
3b6f413 to
5c6cce4
Compare
- Hourly upstream sync from postgres/postgres (24x daily) - AI-powered PR reviews using AWS Bedrock Claude Sonnet 4.5 - Multi-platform CI via existing Cirrus CI configuration - Cost tracking and comprehensive documentation Features: - Automatic issue creation on sync conflicts - PostgreSQL-specific code review prompts (C, SQL, docs, build) - Cost limits: $15/PR, $200/month - Inline PR comments with security/performance labels - Skip draft PRs to save costs Documentation: - .github/SETUP_SUMMARY.md - Quick setup overview - .github/QUICKSTART.md - 15-minute setup guide - .github/PRE_COMMIT_CHECKLIST.md - Verification checklist - .github/docs/ - Detailed guides for sync, AI review, Bedrock See .github/README.md for complete overview Complete Phase 3: Windows builds + fix sync for CI/CD commits Phase 3: Windows Dependency Build System - Implement full build workflow (OpenSSL, zlib, libxml2) - Smart caching by version hash (80% cost reduction) - Dependency bundling with manifest generation - Weekly auto-refresh + manual triggers - PowerShell download helper script - Comprehensive usage documentation Sync Workflow Fix: - Allow .github/ commits (CI/CD config) on master - Detect and reject code commits outside .github/ - Merge upstream while preserving .github/ changes - Create issues only for actual pristine violations Documentation: - Complete Windows build usage guide - Update all status docs to 100% complete - Phase 3 completion summary All three CI/CD phases complete (100%): ✅ Hourly upstream sync with .github/ preservation ✅ AI-powered PR reviews via Bedrock Claude 4.5 ✅ Windows dependency builds with smart caching Cost: $40-60/month total See .github/PHASE3_COMPLETE.md for details Fix sync to allow 'dev setup' commits on master The sync workflow was failing because the 'dev setup v19' commit modifies files outside .github/. Updated workflows to recognize commits with messages starting with 'dev setup' as allowed on master. Changes: - Detect 'dev setup' commits by message pattern (case-insensitive) - Allow merge if commits are .github/ OR dev setup OR both - Update merge messages to reflect preserved changes - Document pristine master policy with examples This allows personal development environment commits (IDE configs, debugging tools, shell aliases, Nix configs, etc.) on master without violating the pristine mirror policy. Future dev environment updates should start with 'dev setup' in the commit message to be automatically recognized and preserved. See .github/docs/pristine-master-policy.md for complete policy See .github/DEV_SETUP_FIX.md for fix summary Optimize CI/CD costs by skipping builds for pristine commits Add cost optimization to Windows dependency builds to avoid expensive builds when only pristine commits are pushed (dev setup commits or .github/ configuration changes). Changes: - Add check-changes job to detect pristine-only pushes - Skip Windows builds when all commits are dev setup or .github/ only - Add comprehensive cost optimization documentation - Update README with cost savings (~40% reduction) Expected savings: ~$3-5/month on Windows builds, ~$40-47/month total through combined optimizations. Manual dispatch and scheduled builds always run regardless.
Review every PR (including drafts) with two jobs that authenticate to AWS Bedrock (Claude Opus 4.8) via GitHub OIDC (vars.AWS_ROLE_ARN); no static AWS credentials are stored in the repo. - ocr-review: runs Alibaba Open Code Review through an ephemeral LiteLLM proxy bridging OCR's OpenAI protocol to Bedrock, and posts inline review comments. Uses output_config.effort=xhigh (Opus 4.8 adaptive thinking). Path-scoped rules (.github/ocr/rule.json) encode PostgreSQL community review standards plus reviewer discipline (verify against the diff, don't hallucinate, state confidence, be blunt, accuracy over approval). - pg-history: OCR cannot call MCP, so a separate Bedrock tool-use agent (.github/ocr/pg-history.py) queries the Agora MCP server (pg.ddx.io) to tie the change to git + pgsql-hackers history, and upserts a comment linking threads as https://pg.ddx.io/m/pgsql-hackers/<message-id>.
The pg-history workflow job has been failing every run with 'Bedrock call failed: The read operation timed out' -- botocore's default 60s read timeout on bedrock-runtime is too short for a multi-round (MAX_ROUNDS=14) tool-use loop against a large PR diff on a reasoning-capable model; a single converse() call alone can take several minutes under load (the sibling ocr-review job's own LLM pass over a similarly large diff took 30-40 minutes). Confirmed via two consecutive live runs against PR #26. Set read_timeout=900s (15 min) explicitly via botocore.config.Config; leave connect_timeout short since a stuck TCP handshake is a different, cheaper-to-detect failure mode that shouldn't wait as long.
github-actions
Bot
force-pushed
the
master
branch
from
ce2678e to
82cc694
Compare
This is the first stage of a native full-text search subsystem intended to
provide true BM25/BM25F relevance ranking with index-only scoring and a
richer query language, addressing long-standing limitations of the
tsvector/tsquery + GIN stack: no corpus statistics (N, avgdl, df) are stored
anywhere, ts_rank is cover-density rather than BM25, and GIN posting lists
carry only TIDs so ranked queries must always recheck the heap.
Rather than land that as one large patch, the work is structured as a
reviewable series (see FTS_NEXTGEN_PLAN.md). This first commit introduces
only the SQL surface, evaluated by sequential scan, with no index access
method -- the same way tsvector/tsquery were originally introduced.
Adds two types:
ftsdoc an analyzed document (sorted, de-duplicated terms with term
frequencies, plus the document length that BM25 will need)
ftsquery a parsed boolean query (AND/OR/NOT and grouping)
Both are varlena and TOAST-able with version-tagged binary send/recv formats.
A hand-written recursive-descent parser produces the query; the grammar is
small enough not to warrant a generator. Matching is a boolean stack machine
over the postfix item list, mirroring TS_execute.
The stage-1 tokenizer is deliberately minimal (ASCII case-fold, split on
non-alphanumerics). It is isolated behind fts_analyze_text() so that a later
stage can reuse PostgreSQL's existing text-search parser and dictionary
pipeline (snowball, ispell, synonyms, thesaurus, stopwords) without changing
the types, the operator, or the on-disk format.
Includes a regression test exercising analysis, query parsing and canonical
output, all boolean match cases, sequential-scan use in a WHERE clause, and
error handling for malformed queries.
Add to_ftsdoc(regconfig, text), which runs an installed text search configuration's parser and dictionary chain via parsetext() and folds the normalized lexemes into an ftsdoc. This reuses the existing snowball/ispell/ synonym/thesaurus/stopword pipeline rather than reimplementing tokenization. Shipped as extension upgrade 1.0 -> 1.1.
Add fts_bm25(doc, query, n_docs, avgdl, dfs), computing the Okapi BM25 score with Lucene-style IDF and standard k1/b defaults. Corpus statistics are caller-supplied for now (the bm25 index AM will maintain them later), which is enough to validate the scoring math by sequential scan. Shipped as 1.1 -> 1.2.
Add a real index access method (USING bm25) over an ftsdoc column that answers the @@@ operator via a bitmap scan. The build scans the heap, collects per-term postings (tid, tf), and writes a metapage (N, sum(doclen), nterms), a sorted dictionary, and chained posting pages -- all through GenericXLog, so the index is crash-safe and replicated without a custom resource manager. The scan evaluates the boolean ftsquery by set algebra over posting lists (AND=intersect, OR=union, NOT=complement against the indexed universe), matching @@@ semantics exactly with no heap access. Corpus statistics are maintained for the coming index-only BM25 scoring. The skeleton is build-once: aminsert raises an error directing REINDEX, and incremental maintenance (pending list + background merge) is a later stage. Shipped as extension upgrade 1.2 -> 1.3.
Add fts_bm25_opts(doc, query, n_docs, avgdl, k1, b, variant, dfs) supporting lucene, robertson (classic), atire, and bm25+ IDF/scoring variants with explicit k1/b, for reproducing reference implementations (Lucene/bm25s) in conformance tests. Shipped as 1.3 -> 1.4.
Add fts_highlight(text, query, pre, post) and fts_snippet(text, query, pre, post, ellipsis, max_tokens), giving FTS5-parity result presentation. Both tokenize the source with the same folding as the analyzer and mark query-term matches; snippet slides a token window and returns the densest match region. Shipped as 1.4 -> 1.5.
Add tsquery_to_ftsquery() and an ASSIGNMENT cast so existing tsquery values and queries port to the @@@ operator with minimal churn: &/|/! map to AND/OR/NOT. The phrase operator <-> degrades to AND with a NOTICE (phrase support is a later stage), preserving recall. Shipped as 1.5 -> 1.6.
Add prefix matching to the query language: a trailing '*' on a term (e.g. quick*) matches any document term with that prefix. Implemented in the parser (a per-item FTS_QF_PREFIX flag, carried through send/recv), the sequential matcher (binary-search lower bound on the sorted term set), and the bm25 index scan (union the posting lists of all dictionary terms sharing the prefix). Phrase and NEAR need per-term positions, which the stage-1 ftsdoc format omits; they follow as an ftsdoc v2 format addition.
Add fts_index_stats(regclass) -> (ndocs, avgdl, nterms) and fts_index_df( regclass, ftsquery) -> float8[], reading N, avgdl and per-term document frequency from the bm25 index metapage and dictionary. BM25 can now be scored from statistics the index maintains rather than caller guesses, closing the loop between the AM and the scorer. (Streaming index-only WAND top-K is a further optimization.) Shipped as 1.6 -> 1.7.
…partial) Update the README to reflect the nine qualified stages (versions 1.0-1.7 plus prefix queries) and to state honestly what remains: phrase/NEAR, WAND top-K, incremental maintenance, contentless indexes, the parity gate, and the fuzzy/regex stages.
The bm25 access method's aminsert no longer errors: it appends the new document verbatim to an in-index chain of pending pages and bumps the metapage N and sum(doclen). The scan searches pending documents directly with the per-document matcher, so newly inserted rows are immediately visible to @@@ without a REINDEX. Per-term df in the dictionary stays stale until a merge (REINDEX), matching GIN fastupdate's documented behavior. All page writes go through GenericXLog. Shipped as 1.7 -> 1.8 (bm25 metapage format changed; REINDEX required for pre-1.8 bm25 indexes).
Extend the ftsdoc format to v2, storing per-term token positions, and add
quoted-phrase query syntax ("a b c"): the parser emits an FTS_OP_PHRASE chain
(distance 1), and the matcher verifies adjacency by intersecting term position
lists. The bm25 index treats a phrase as AND for candidate generation and now
requests a bitmap recheck, so @@@ re-evaluates adjacency exactly against the
heap ftsdoc. Position-free v1 docs remain valid (phrase degrades to AND).
NEAR(a b, k) reuses the same distance-aware phrase_step and is a small parser
addition (comma + integer) on top of this. Shipped as 1.8 -> 1.9 (ftsdoc
format v2).
The bm25 index stores only postings, never document text, so an expression index on to_ftsdoc(text_column) is exactly FTS5's external-content model: the text lives in the base table, the index is derived from it, and @@@ queries (including phrases, via recheck) work against the expression. Shipped as 1.9 -> 1.10 (documentation marker; no new SQL objects).
Add two ftsquery term forms:
term~k matches document terms within Levenshtein distance k (default 2),
using core varstr_levenshtein_less_equal (bounded, no new dependency)
/re/ matches document terms against a POSIX regex via core's cached
regex engine
Both are evaluated per-document in the matcher. The bm25 index returns all
indexed tuples as candidates for fuzzy/regex queries and the bitmap heap
recheck applies the exact test, so results are correct through the index.
This follows the plan's 'no new dependency for the common case' path; the
pg_tre trigram-formula pre-filter (with its Lime grammar converted to
Bison+Flex, and sparsemap v5.1.1 for posting compression) to narrow candidates
at scale is future work. Shipped as 1.10 -> 1.11.
Add bench/bench.sql and bench/README: a reproducible A/B harness comparing the bm25 stack against tsvector + GIN + ts_rank on a user-supplied corpus (index size, ranked top-10 EXPLAIN ANALYZE). The full parity gate (latency percentiles, NDCG vs qrels, concurrent-ingest throughput, Lucene/bm25s score conformance) is documented as a manual, reported measurement rather than a make-check regression, since it needs an external corpus. Update README.pg_fts to describe all implemented stages (1.0-1.11), the full query language, a worked BM25 ranking example, and the remaining future work (WAND top-K, trigram pre-filter for fuzzy/regex, NEAR, background merge, BM25F).
Add fts_bm25f(docs ftsdoc[], query, weights[], n_docs, avgdls[], dfs[]): the Robertson/Zaragoza BM25F, where per-field term frequencies are length- normalized per field and combined by weight before the tf-saturation step (not a naive sum of per-field BM25 scores). This lets a term in a heavily weighted field (e.g. title) outrank the same term in the body. Shipped as 1.11 -> 1.12.
Add bm25_merge_pending(): read the existing dictionary + posting chains and all pending documents back into a build state, rewrite the merged structure into fresh blocks, and repoint the metapage -- no heap access. Wired into amvacuumcleanup (VACUUM now folds pending docs automatically) and exposed as fts_merge(regclass) for on-demand merge. Merging resolves the df staleness that incremental inserts introduce (formerly-pending terms gain dictionary df). Old blocks are left unreferenced and reclaimed by REINDEX; an FSM-based page recycler is future work. Shipped as 1.12 -> 1.13.
Add fts_search(index, query, k) -> setof(ctid, score): BM25 top-k computed entirely from the index -- postings supply per-doc tf, the dictionary supplies df and a per-term max-tf impact bound (now stored), the metapage supplies N and avgdl -- with no heap access. Per-document scores accumulate across query terms and the top-k are returned by descending score; join on ctid to fetch rows. This is the index-only-scoring path (no heap fetch to rank), the core performance win for ranked search. Stored per-term max_tf in the dictionary provides the WAND upper bound for document skipping. Full executor integration via amcanorderbyop (an ORDER BY score LIMIT k ordering scan with block-max WAND early termination) and exact per-document |D| in postings are the remaining optimizations. Shipped as 1.13 -> 1.14.
Add pg_fts_trgm.c: reduce a fuzzy term to its trigrams and test only document terms sharing a trigram with it (Levenshtein is the expensive step). This is the pg_tre-style pruning that makes fuzzy matching viable on a large vocabulary, applied at the term level. Results are unchanged: the filter only skips candidates that provably cannot match (pigeonhole: a match within k edits shares a trigram when the term has more than k trigrams) and falls back to a full scan for short terms. A persistent on-disk trigram posting index in the bm25 AM (the full three-tier funnel) is the remaining scale work. Shipped as 1.14 -> 1.15.
fts_search() returned candidate ctids straight from the postings, which can reference dead or updated tuples that the index has not yet merged out. It now opens the base table and checks each candidate against the active snapshot via table_index_fetch_tuple, returning only visible tuples in score order and stopping once k visible rows are found. This makes the SRF correct under all isolation levels, matching the visibility contract the @@@ bitmap path already gets from the executor's bitmap heap scan + recheck. (The @@@ operator path was already MVCC-correct: amgetbitmap sets recheck=true and the bitmap heap scan applies snapshot visibility. All page I/O uses the buffer manager, and every writer is WAL-logged via GenericXLog, so the index is correct on physical standbys and after crash recovery.)
…t fix) The previous fix missed the first gate-off site in __sm_map_unset (the 'no chunks in the map' branch still assigned SM_IDX_MAX to the size_t offset), so the 32-bit tombstone VACUUM crash persisted. Route that site through SM_UNSET_NO_COALESCE too, and change __sm_chunk_rank's rank->rem 'infinity' sentinel from UINT64_MAX to SIZE_MAX (rank->rem is size_t; the value is only ever used as a large remaining-count, but the constant must not truncate). Verified on genuine i686 (gcc -m32 on Fedora): the delete-all-then-reuse coalescing reproducer passes and the build is free of -Woverflow/-Wuninitialized in sm.c. 64-bit upstream suite still green (test_main 44/44, coverage 7801, portability, RLE); pg_fts regression green.
Real-corpus testing exposed a correctness bug: after a document is deleted and VACUUMed (tombstoned in its segment), if its heap slot is reused by a NEW inserted document (same TID/docid), the new document was not found by @@@ / fts_count. Two causes, both fixed: 1. Pending docs were tombstone-filtered. A doc in the pending write buffer is a live heap tuple by definition, but bm25_collect_matches OR'd pending matches into the accumulator BEFORE the tombstone filter, so a pending doc reusing a tombstoned TID was dropped. Collect pending matches separately and union them AFTER filtering; pending docs are never tombstoned. 2. Tombstones were applied globally across segments. bm25_docid_tombstoned checked a docid against EVERY segment's tombstone map, but a tombstone belongs to exactly one segment: a TID deleted in segment A may be legitimately reused by a live doc that a newer segment B indexes. Filter each segment's own match contribution against only THAT segment's tombstone map (bm25_filter_tombstoned_seg), and in the WAND ranked path skip own-segment tombstoned docids inside each cursor (cursors are per-segment). Removed the now-unused global bm25_filter_tombstoned / bm25_docid_tombstoned. The regress tombstone test previously encoded the buggy 0 as expected output; corrected to 60 and added an fts_count assertion so the count path is covered too. qualify PASS; regression green.
A document whose analyzed ftsdoc did not fit on a single pending page raised 'ftsdoc too large for a bm25 pending page' and failed the INSERT/UPDATE -- real corpora (e.g. long Wikipedia articles) hit this routinely, so the index could not be built over them. When an ftsdoc exceeds the pending-page capacity, index it directly as its own one-document segment (bm25_insert_oversized_as_segment) via the existing build machinery: segment posting storage is a chain of FOR-packed pages with no per-document size limit. Such documents are rare, so building a small segment per oversized insert is acceptable; corpus N/sumdoclen stay correct (bm25_meta_add_segment accounts the doc, and the pending path is bypassed so there is no double count). Added a regression test with a ~4000-token document.\n\nqualify PASS; regression green.
Vendor sparsemap v5.2.0, which brings the MSVC portability shim (SM_ALIGNED / ssize_t / guarded __builtin_*) and the O(N) coalescing performance fix upstream. This replaces the local vendor/sm_compat.h shim (now redundant) and my earlier one-off MSVC edits; sm.h is byte-identical to pristine v5.2.0 and sm.c differs only by the __pg_bm25_ SPARSEMAP_PREFIX namespacing block plus the fix below. v5.2.0 still has the 32-bit unset-coalesce truncation bug (its own change did not touch the gate): in __sm_map_unset the size_t byte-offset 'offset' is set to SM_IDX_MAX (== UINT64_MAX) as a 'gate coalescing off' sentinel, which truncates to 0xFFFFFFFF on ILP32 so the '!= SM_IDX_MAX' gate (which promotes offset back to 64 bits) never matches -> coalescing runs on an uninitialized chunk and crashes. Route the four gate-off sites and the gate test through a size_t-width sentinel SM_UNSET_NO_COALESCE. (Fixed in ~/ws/sparsemap for upstreaming; vendored here.) Consumer-side alignment fix: struct sparsemap is declared SM_ALIGNED(8), but palloc only guarantees MAXALIGN (4 on ILP32), so palloc0(n*sizeof(sm_t)) placed the per-segment tombstone maps at 4-aligned addresses -> -fsanitize=alignment abort in the Linux Meson (32-bit) CI job. Allocate that array with palloc_aligned(..., 8, 0). Verified on genuine i686 under -m32 -fsanitize=undefined,alignment: the delete+coalesce+reopen path runs clean (live=60) with an 8-aligned sm_t. qualify PASS; 64-bit regression green; upstream suite (test_main 44/44, coverage 7805, portability, RLE) green.
Indexing an oversized document creates a one-document segment (bm25_insert_oversized_as_segment). A bulk INSERT/UPDATE over a corpus with many large documents (e.g. rebuilding the expression index over 2M Wikipedia rows) could therefore create one segment per oversized row and hit the hard BM25_MAX_SEGMENTS (128) cap before the next VACUUM merged anything -- 'bm25 index reached the maximum of 128 segments'. After flushing an oversized segment, if the live segment count is within 16 of the cap, run bm25_merge_segments() to coalesce. Verified: 200 oversized documents now index without overflow (merged to well under the cap) and all remain searchable. qualify PASS; regression green.
Head-to-head on 2,000,000 real English-Wikipedia articles (PG20devel, r7i.8xlarge). Match sets verified identical to the GIN path before timing. Headline (median of 9, warm): ranked retrieval -- the actual FTS use case -- is where pg_fts wins, and the win grows with term frequency because GIN's ts_rank must fetch+score+sort every match while pg_fts stops early via block-max WAND: ranked top-10 common&mid 15 ms vs 65 ms (4.2x), top-100 common 75 ms vs 3029 ms (40x), top-10 two-common 50 ms vs 1641 ms (33x). Plain counts / boolean AND tie (both bitmap-scan); fts_count beats count(*) 3.7x. Cost: ~2.5x larger index (per-posting tf/|D|/positions), comparable single-threaded build. Adds bench/get_wikipedia.py (HF parquet -> TSV loader) and bench/bench_fixed.sh (pinned-term median-of-9 A/B runner).
…ch bench sparsemap v5.2.1 upstreams the ILP32 gate fix I carried locally in the vendored copy (SM_UNSET_NO_COALESCE, the size_t-width coalesce sentinel) and adds an independent bug fix -- a multi-run RLE-chunk corruption in __sm_separate_rle_chunk (wrong payload-vector placement and pivot-size accounting that desync'd the sequential chunk walk). Re-vendor v5.2.1: sm.h is byte-identical to pristine v5.2.1 and sm.c now differs ONLY by the __pg_bm25_ SPARSEMAP_PREFIX block (no local modifications remain). Verified on i686 (gcc -m32 -fsanitize=undefined, alignment): the delete+coalesce+reopen path is clean; 64-bit upstream suite green (test_main 44/44, coverage 7820). qualify PASS; regression green. Also add bench/RESULTS_VS_PGSEARCH_WIKI.md: an honest pg_search 0.24.1 head-to- head on 2M real Wikipedia articles (PG17.10). pg_search wins ranked retrieval (flat ~9ms; pg_fts's docid-ordered WAND degrades with term frequency, 26->70ms) and common-term counts (aggregate pushdown); pg_fts's fts_count wins selective counts (1.9-2.4ms) and its index is 1.55x smaller (3590 vs 5574 MB). The gaps are the documented codec investments (impact-ordered postings, COUNT/aggregate Custom Scan pushdown) plus parallel scan -- architecture, not tuning.
…AIO) Code-cited capability matrix and answers to adopter questions: concurrent builds (CIC/REINDEX CONCURRENTLY work — aminsert routes concurrent writes to the searchable pending list), index-only scans (no, non-covering by design; fts_count is the fast-count path), compaction (VACUUM+fts_merge drops tombstones logically, REINDEX reclaims physical space — no online REPACK), feature parity vs pg_search/Tantivy, ZomboDB/ES and tsvector/GIN (has BM25/BM25F, phrase/NEAR/ prefix/fuzzy/regex, <=> ordering scan, fts_count, highlight/snippet, tombstone deletes, WAL/GenericXLog safety; gaps: no parallel build/scan, no IOS, no aggregation pushdown, no impact-ordered postings), logical-replication drop-in (no — re-platform: different API, indexes provisioned per-subscriber; physical replication + crash recovery ARE safe), and AIO (none of its own; build heap scan gets core read_stream free; nextblk pointer-chains defeat prefetch and WAND is anti-prefetch by design; only the cold merge full-scan could benefit, deferred until measured).
Closes the ranked-retrieval gap vs Tantivy/pg_search, whose latency stays flat as term frequency grows while pg_fts's docid-ordered block-max WAND degraded (top-100 over a common term walked the whole ~5300-block posting list). Format v3 adds a per-term impact-ordered block skip directory: for a term with df >= BM25_SKIPDIR_MIN (2048), the writer records every posting block's (blk, off, max_tf, min_doclen) and stores them sorted DESCENDING by an avgdl-independent impact proxy (max_tf desc, min_doclen asc) in a BM25_SKIPDIR page chain, referenced from three new BM25DictEntry fields (skipstart/skipoff/ nblocks). The single-term ranked scan (fts_search_impact_single, dispatched from fts_search_wand when nterms==1 and a directory exists) visits blocks best-first and stops once k results beat the recomputed bound of the next block -- Tantivy-style early termination. Soundness (where the prior float-impact attempt failed on avgdl drift): only raw integers are stored; the bound is RECOMPUTED at query time from (max_tf, min_doclen) at the current avgdl, so it is always an exact upper bound as the corpus grows. The stored sort order is a visitation heuristic only -- exactness comes from the recomputed bound + the WAND stop condition. Verified: the index top-k set is identical to a brute-force seqscan+sort (regression test with a >2048-doc term and distinct top scores). Rare terms carry no directory and use the existing exact docid-ordered scan; v2 indexes are read with that scan too (version-gated), so no forced rewrite -- REINDEX gains the directory. Format BM25_VERSION 2->3; extension 1.19->1.20 (migration is a marker, no SQL surface change). qualify PASS; regression green.
The skip directory is stored sorted by a max_tf proxy, but the true per-block impact bound also depends on min_doclen (impact rises as min_doclen falls), so a block with a smaller max_tf can have a higher bound than an earlier entry. Stopping at the current entry's bound was therefore unsound -- it could halt before a later, higher-bound block, returning a wrong/incomplete top-k (observed at 2M scale: index top-k distances non-ascending and missing lower-distance docs vs a seqscan). Fix: after loading a term's directory, sort it by the EXACT impact bound recomputed at the current avgdl (descending) before the scan. Then the front-of-remaining early-stop is a valid ceiling on every not-yet-visited block and the returned top-k is exact. Verified with distinct scores at 8000 docs: index top-15 byte-identical to a brute-force seqscan+sort. (Insertion sort; near-O(n) since the stored max_tf order is already close to bound order.)
The k*4 (min 64) over-fetch forced the impact-directory single-term scan to fetch far more than the LIMIT (a LIMIT 10 fetched 64), defeating its early stop. k*2 (min 32) still tolerates ~50% invisible top rows before the SRF under-fills, and the amgettuple ordering scan grows k via its own retry, so correctness holds while the early-stop can fire. qualify PASS; regression green.
… real text) The v3 impact-ordered block skip directory (commits f049f3a/ef637ba/68ce28f) was correct and avgdl-drift-safe, but instrumented block-visit counts on 2M real Wikipedia show it delivers NO early termination: within a term the per-block impact bounds cluster in a razor-thin band just above the top-k threshold (constant idf; a common term has some high-tf doc in nearly every block), so best-first block ordering still visits ~99% of blocks before it can stop. For 'year' (df 678k, 5296 blocks) the scan visited 5282; for 'hungary' 170/173. No measured latency win on any query band, at the cost of format v3, a skip-page chain, ~3% larger index and a per-query sort. Reverting to format v2 / extension 1.19. pg_search's flat ranked latency comes from a compact columnar segment codec (far less decode per candidate) plus query parallelism, not an impact skip structure -- matching it is a codec rewrite, out of scope here. bench/NOTE_IMPACT_ORDERING.md records the attempt, the measurements, and the conclusion so it is not re-tried blindly. qualify PASS; regression green; index format unchanged (v2).
…cation) The regression suite covered functionality but not the harder committer bars. Add: - specs/bm25_concurrency.spec: MVCC/concurrency invariants under the isolation tester -- REPEATABLE READ snapshot stability across a concurrent committed INSERT, the pending list being live for a fresh snapshot, a concurrent VACUUM/fts_merge being MVCC-invisible to an open scan, and delete+VACUUM+ heap-slot-reuse leaving no resurrected/hidden docs (per-segment tombstones). - specs/bm25_cic.spec: CREATE INDEX CONCURRENTLY and REINDEX CONCURRENTLY complete and reflect a concurrently-committed INSERT (aminsert routes to the searchable pending list). - t/001_crash_recovery.pl: an immediate (crash) stop + automatic recovery reproduces exact @@@/fts_count/ranked answers across the build, pending-list, and tombstone paths, and the index is writable afterward -- proving the GenericXLog WAL logging is crash-safe. - t/002_replication.pl: the index replicates to a streaming standby and returns identical @@@/<=>/fts_count results there, including tombstoned deletes. Wired into meson.build (isolation specs + tap tests) and Makefile (ISOLATION, TAP_TESTS=1). All green: isolation 2/2 specs, crash recovery 5/5, replication 5/5; qualify PASS.
…ADME The extension had no user-facing reference docs -- a required gap for a contrib module. Add doc/src/sgml/pgfts.sgml (the 'pg_fts -- BM25 full-text search' appendix section: data types, @@@ / <=> operators, the bm25 index, functions to_ftsdoc/to_ftsquery/fts_count/fts_search/fts_merge/fts_bm25[f]/fts_index_*/ fts_highlight/fts_snippet/tsquery_to_ftsquery, an example, and limitations), and wire it into filelist.sgml (entity) and contrib.sgml (alphabetical include). Also add Documentation and Testing sections to README.pg_fts pointing at the SGML docs, CAPABILITIES.md, and the regression/isolation/TAP/bench suites, so the test and doc structure is discoverable. SGML tags balanced, entities all standard; qualify PASS; regression + isolation green.
v5.3.0 adds accelerated membership APIs; adopt the two that fit pg_fts's tombstone lookups, preferring stack-allocated cache state to avoid palloc: - bm25_filter_tombstoned_seg: replace the per-docid sm_contains loop with a single sm_contains_many() left-to-right sweep (O(chunks+n) instead of n head-walks). The set's docids are already ascending (TidSet is TID-sorted, docid is monotonic in TID), satisfying sm_contains_many's precondition. Uses a 256-entry stack buffer for the common small case, palloc only for larger sets. - wand_cur_own_tombstoned (WAND ranked hot loop): use sm_contains_cached() with a stack-allocated sm_cursor_cached_t carried on the WandCursor (8-way MRU chunk cache), initialized once per cursor. A cursor scans docids ascending into a small working set, so repeated lookups become cache hits. - bm25_read_segment_into (merge): use a stack sm_cursor_cached_t across a term's docid-ascending postings. (The merge's dead-map dedup check stays plain sm_contains -- that map is mutated in the loop, which the cached API forbids.) sm.h is byte-identical to pristine v5.3.0; sm.c differs only by the __pg_bm25_ prefix block. All tests green: regression, isolation 2/2, crash-recovery 5/5, replication 5/5; qualify PASS.
pg_fts's fts_bm25_opts offered lucene/robertson/atire/bm25+. The rank_bm25 /
plpgsql_bm25 reference set also includes BM25L, which shifts the LENGTH-
NORMALIZED term frequency by a delta before saturation
(ctd = tf/(1-b+b*dl/avgdl); sat = (k1+1)(ctd+d)/(k1+ctd+d), IDF ln((N+1)/(df+0.5)),
delta 0.5) so long documents are not over-penalized -- a distinct behavior from
BM25+ (which adds the delta to the whole tf-saturation term). Add BM25_BM25L
to the scorer and accept the names 'bm25l' and 'l'. With this the variant set
matches rank_bm25's ('', l, plus, robertson, luceneaccurate, atire, bm25l,
bm25plus map onto lucene/robertson/atire/bm25+/bm25l). Regression test +
docs updated; qualify PASS, regression + isolation green.
The single-threaded CREATE INDEX was pg_fts's weakest axis (build is dominated
by to_ftsdoc tsearch analysis; 2M Wikipedia took ~34 min serial while pg_search
built in ~1 min with parallel workers). Implement Level-1 parallel build,
modeled on BRIN's parallel-build scaffolding:
- amcanbuildparallel = true. When the planner sets ii_ParallelWorkers > 0,
bm25_build sets up a ParallelContext + DSM (BM25Shared: heap/index oids,
isconcurrent, a parallel table scan, a done-counter, buffer/WAL usage), and
registers bm25_parallel_build_main as the worker entry ('pg_fts' library).
- Each participant (leader + workers) runs table_index_build_scan over its
slice of a shared parallel table scan, so the expensive heap-scan + analysis
parallelizes near-linearly, and flushes its accumulated postings as segments
straight into the index. The leader then runs the size-tiered merge to
compact the workers' segments.
- Segment WRITES are serialized: pg_fts appends pages via ReadBuffer(P_NEW),
and overlapping appenders race on relation extension ('unexpected data beyond
EOF'). bm25_build_flush_segment holds the relation extension lock around the
whole segment write when IsInParallelMode(); the analysis stays fully
parallel, only the cheap write phase serializes. (Serial builds take no lock.)
Verified: a parallel-built index returns byte-identical counts and the same
ranked top-k as a serial build over the same data; corpus stats (N) match.
Regression test forces workers (min_parallel_table_scan_size=0); isolation +
crash + replication tests unaffected. qualify PASS; regression + isolation
green. Docs/capability matrix updated (build now parallel; query scan still
single-threaded).
…8+ (3) The flags parameter to table_beginscan_parallel was added after PG17; gate the SO_NONE argument behind PG_VERSION_NUM >= 180000 so the parallel build compiles on PG17 as well as master.
Level-1 parallel build parallelized the heap scan + analysis, but the build-time bm25_merge_segments (merge everything to one segment) is single- threaded and O(index); with N workers each flushing several segments it dominated, so the net build speedup was only ~15%. After a PARALLEL build, do not force a full merge: leave the participants' segments in place (the index is fully correct as N segments) and let the size-tiered background merge (VACUUM / fts_merge) coalesce them lazily, as Lucene/Tantivy parallel builds do. Only run a single tiered-merge pass if the count exceeds BM25_MERGE_THRESHOLD, to bound scan fan-out. Serial builds keep the cheap post-build compaction (they produce few segments). Future (noted in code): Level-2 -- parallelize the merge itself (workers merge disjoint segment groups) to also cut compaction cost; deferred. qualify PASS; regression + isolation green.
fts_merge() only called bm25_flush_pending, which returns immediately when the pending list is empty -- so it did nothing for an index that has many segments but no pending docs (exactly the state a parallel build leaves, since that skips the build-time merge). Add bm25_merge_all() -- merge every live segment into one, looping in bounded batches -- and have fts_merge() run it after flushing. An on-demand fts_merge() now yields an optimal single-segment index; the tiered auto-merge (which deliberately leaves same-size tiers) is unchanged for the background path. Verified: a 2-segment parallel-built index fts_merge()s to 1 with counts intact. qualify PASS; regression + isolation green.
Compacting a many-segment index (e.g. the state a parallel build leaves) was a single-threaded O(index) decode+re-encode. Parallelize it: bm25_merge_all now first runs bm25_merge_all_parallel, which partitions the live segments into (workers+1) disjoint groups, has each participant merge ONE group into a new segment via bm25_merge_group_to_seg (writes pages only, extension-lock- serialized, no directory touch), and then performs a SINGLE atomic metapage update -- dropping every consumed source (content-match, not index, so it is robust to the directory having changed) and installing the per-group merged segments, recycling the sources' pages. A cheap final serial pass finishes to one segment. This confines the expensive per-segment decode/re-encode to parallel workers and keeps the directory swap serial + atomic (no concurrent- swap race). Nested parallelism is avoided (skipped when IsInParallelMode()). Worker entry bm25_parallel_merge_main registered under 'pg_fts'. Verified: a parallel-merged index returns byte-identical counts to a serial build and to the pre-merge index (common/term/alpha counts across 100k-120k rows), merges to a single segment, ranked top-k intact, no crash. Future (noted): Level-2 recursive parallel merge (W->W/2->...->1) so the final combine also parallelizes. qualify PASS; regression + isolation green.
Record the enhancements discussed during development but not yet implemented so they are not rediscovered: parallel-merge at-scale timing + worker-slot gating, Level-2 recursive parallel merge, larger per-worker build segments, impact- ordered postings / columnar codec (the ranked-latency gap vs pg_search), COUNT Custom Scan pushdown, parallel scan, cold-merge AIO prefetch, benchmarking the v5.3.0 batch/cached sparsemap APIs under churn, completing the 4-way (pg_fts/pg_search/VectorChord-bm25/tsvector-GIN) comparison, fts_search SRF under-fetch safety, sparsemap error-path leaks, and the release tag decision.
github-actions
Bot
force-pushed
the
master
branch
from
82cc694 to
43b209f
Compare
…regression) The parallel build (commit 019bd1f) skipped the final merge, leaving a freshly built or REINDEXed index as N segments (~6-8, ~8-13 GB with unreclaimed pages). A multi-segment index makes ranked scans traverse every segment's postings, so common-term ranked top-k regressed ~2x (18 ms -> 38 ms at 2M Wikipedia) versus the old serial build's single-segment index. Fix: after bm25_end_parallel() (which has exited parallel mode), call bm25_merge_all(), which now runs the PARALLEL merge (workers merge disjoint segment groups) -- so the build ends with an optimal single segment WITHOUT the single-threaded O(index) merge tail that made a naive build-time merge slow. Verified locally: a 400k-row parallel build ends nseg=1 with 18 parallel-merge worker launches, counts correct, no crash. qualify PASS; regression + isolation green.
Ensure BOTH build paths leave an optimal single-segment index: the serial branch used the tiered bm25_merge_segments, which deliberately leaves same-size tiers and can leave a multi-segment index (regressing ranked scans). Use bm25_merge_all in both branches so a fresh CREATE INDEX / REINDEX is always compact regardless of whether the planner chose a parallel build. qualify PASS; regression + isolation green.
github-actions
Bot
force-pushed
the
master
branch
2 times, most recently
from
9f8691a to
6a5068d
Compare
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.
No description provided.