Skip to content

feat: first-class bm25 ranked-retrieval index - #25584

Closed
cpegeric wants to merge 956 commits into
matrixorigin:mainfrom
cpegeric:bm25_retrieval
Closed

feat: first-class bm25 ranked-retrieval index#25584
cpegeric wants to merge 956 commits into
matrixorigin:mainfrom
cpegeric:bm25_retrieval

Conversation

@cpegeric

@cpegeric cpegeric commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #25541

What this PR does / why we need it:

Summary

Add bm25 as a new first-class secondary index type in MatrixOne: a lean,
position-free BM25 ranked-retrieval index for text columns, decoupled from the
classic fulltext index. It answers disjunctive (OR) top-K bag-of-words queries with
Block-Max WAND, has its own query verb BM25(col) AGAINST('query'), and is maintained
incrementally via CDC (async) with scheduled tiered-merge compaction.

Unlike the classic fulltext index (which stores term positions to serve boolean /
phrase / natural-language-with-expansion modes), bm25 is intentionally
position-free. That makes its on-disk index much smaller and its top-K walk much
faster, at the cost of not supporting operator/phrase queries — the two index types are
complementary and can coexist on the same column.

Motivation

  • The classic fulltext index pays for term positions on every posting to support
    boolean/phrase modes. A large class of retrieval workloads only need ranked
    bag-of-words top-K
    (search boxes, RAG retrieval, "most relevant N documents").
  • For that workload a position-free BM25 index with Block-Max WAND skips
    non-competitive documents and returns the true top-K without scoring the whole
    postings list. Benchmarks showed ~5× faster query latency than the classic
    fulltext path on the same corpus.

Query surface

-- ranked top-K (BM25 score DESC); LIMIT is pushed into the WAND walk
SELECT id FROM docs WHERE BM25(body) AGAINST('influenza vaccine') LIMIT 10;

-- score in the projection
SELECT id, BM25(body) AGAINST('influenza vaccine') AS score
FROM docs WHERE BM25(body) AGAINST('influenza vaccine') ORDER BY score DESC;

-- filtered retrieval (predicate pushed into the walk as a membership bitset)
SELECT id FROM docs
WHERE BM25(body) AGAINST('influenza vaccine') AND category = 10 LIMIT 10;

BM25() is a distinct verb from MATCH():

  • BM25(col) AGAINST('q') → the bm25 index. Mode-free (always ranked bag-of-words).
  • MATCH(col) AGAINST('q' [IN … MODE]) → the classic fulltext index (unchanged).

The two are disambiguated by function name alone, so a column can carry both index
types and each verb targets its own — no mode heuristics.

DDL

CREATE INDEX ftx USING bm25 ON docs(body) WITH PARSER gojieba;

-- optional async / scheduled-compaction knobs
CREATE INDEX ftx USING bm25 ON docs(body)
  WITH PARSER gojieba, ASYNC, AUTO_UPDATE, HOUR 3, MAX_INDEX_CAPACITY 1000000;

ALTER TABLE docs ALTER REINDEX ftx BM25 MERGE;    -- incremental tiered merge
ALTER TABLE docs ALTER REINDEX ftx BM25;          -- full rebuild-from-source
DROP INDEX ftx ON docs;
  • Single text column (CHAR/VARCHAR/TEXT/JSON/DATALINK).
  • WITH PARSER selects the tokenizer: gojieba (jieba dict + per-index overflow
    vocab), ngram, default.
  • Follows the vector-index DDL path (USING <algo>*tree.Index
    BuildSecondaryIndexDefs), not the fulltext CREATE FULLTEXT INDEX path.

Design

Engine (pkg/bm25/wand)

  • Block-Max WAND (position-free, doc-ordered, disjunctive top-K) with a
    membership allow-set consulted during the walk for filtered search.
  • Two-tier LSM: tag=0 base sub-indexes (capacity-bounded, recency-ordered) +
    tag=1 CdcTail (delta insert frames + delete frames). Liveness resolved by recency.
  • Serialization keys documents by primary key (encodePk/decodePk); a normalizeKey
    canonicalizes string-family pks so they can key the liveness/dedup maps.

Plugin (pkg/bm25/plugin) — via the indexplugin framework

  • runtime (catalog hooks): two hidden tables [storage, metadata], AlwaysAsync,
    SyncDescriptor (CDC + idxcron action bm25_reindex, option MERGE),
    ParamsFromTree (parser / async / auto_update / day / hour / second /
    max_index_capacity).
  • compile: sync build-from-source or async CDC-arm; HandleReindex (REBUILD/MERGE);
    restore InitSQL; ValidateReindexParams.
  • plan: BuildSecondaryIndexDefs (storage + metadata tables, no postings table);
    inert CanApply/ApplyForSort (bm25 uses the BM25() path, not ORDER-BY rewrite).
  • idxcron: tail-threshold Updatable; ReindexOption MERGE-vs-REBUILD by dead-doc %.
  • iscp: CDC writer = the WAND sql-writer.
  • Blank-imported in pkg/indexplugin/all + pkg/indexplugin/iscp.

Hidden tables

  • storage — chunked binary (WAND) index blobs (index_id, chunk_id, data, tag).
  • metadata — one row per sub-index (index_id, timestamp, checksum, filesize, recency, nrow).
  • No postings table — bm25 builds directly from the source rows.

Table-valued functions

  • bm25_search(param, cfg{db,index,metadata}, pattern) — tokenize + Block-Max WAND
    top-K, emitting (doc_id, score); honors a runtime-filter membership bitset and
    pushed-down LIMIT.
  • bm25_create — build tag=0 base from source.
  • bm25_compact — fold tag=1 tail into tag=0 (tiered merge).

Query planning (pkg/sql/plan/apply_indices_bm25.go)

BM25(col) AGAINST('q') binds to the bm25_match function → rewritten to a
bm25_search TVF INNER-JOINed to the source on pk, with a SORT by score DESC and
LIMIT pushdown. When combined with a non-MATCH filter and
fulltext_bloom_filter_pushdown=ON, a pre-filter join builds a membership bitset that
is pushed into the WAND walk so only qualifying documents are scored.

CDC + scheduled compaction

  • Post-create DML flows into the tag=1 CdcTail via the ISCP WAND sinker (async).
  • idxcron (cluster-singleton) periodically fires an ALTER REINDEX … MERGE to fold
    the tail into the base. Interval knobs: DAY/HOUR/SECOND per-index +
    MO_IDXCRON_TICK_SEC / MO_IDXCRON_CRON / MO_IDXCRON_INTERVAL_SEC envs.

Primary-key support

encodePk covers: int32/64, uint32/64, varchar/char/text/datalink, uuid,
date/datetime/time/timestamp, decimal64/128, and composite pk (packed
CPrimaryKey varchar). Any other single-column pk type
(tinyint/smallint/float/bit/bool/enum) is rejected at CREATE INDEX with a clear
message (rather than silently aborting CDC later).

Lifecycle covered

CREATE (sync + async) → query (BM25(), score, LIMIT pushdown, filtered) → live DML via
CDC → ALTER REINDEX (REBUILD / MERGE) → scheduled idxcron MERGE → CLONE → RESTORE →
DROP.

Testing

  • Engine unit (pkg/bm25/wand) — build / search / merge / serialize / tail /
    deletes / compaction (incl. string-pk MERGE regression).
  • Plugin unit (pkg/bm25/plugin/{plan,runtime}) — schema build, pk-type
    validation, param extraction, sync descriptor.
  • BVTtest/distributed/cases/fulltext/bm25_* (retrieval, datetime, uuid,
    multibase, limit, mode_guard, coexist, pushdown) and
    test/distributed/cases/pessimistic_transaction/bm25/* (basic, async, reindex,
    merge, clone, restore).

Non-goals / known limitations

  • Position-free: no boolean / phrase / natural-language-with-expansion. Use a
    classic fulltext MATCH index for those (the two coexist).
  • A single query combining BM25() and MATCH() on the same statement is rejected
    (clear error) — run them as separate queries.
  • Cross-CN cache freshness after a CDC update is bounded by the shared index-cache TTL
    (same model as the vector indexes).

Acceptance criteria

  • CREATE INDEX … USING bm25 … WITH PARSER <tok> builds storage + metadata tables.
  • BM25(col) AGAINST('q') returns ranked top-K; LIMIT pushed into the walk.
  • Post-create DML converges via CDC; ALTER REINDEX … MERGE folds the tail.
  • Filtered retrieval (… AND <filter>) with pushdown returns identical rows to
    the non-pushdown path.
  • Coexist: MATCH→classic, BM25→bm25 on the same column.
  • Clone / restore reproduce a queryable index; idxcron MERGE fires on schedule.
  • Unsupported pk types rejected at CREATE; supported pk types (incl. string /
    uuid / datetime / decimal / composite) round-trip through MERGE.
    eric@mo matrixone % cat ../../../../private/tmp/claude-501/-Users-eric-github-matrixone/3ea221e9-92a9-4e29-a7b4-ae404b34ac35/scratchpad/bm25_index_pr.md

feat: first-class bm25 ranked-retrieval index

What

Adds bm25 — a new first-class, position-free BM25 ranked-retrieval index for
text columns, with its own query verb BM25(col) AGAINST('query'), incremental CDC
maintenance, and scheduled tiered-merge compaction. It is decoupled from the classic
fulltext index and complementary to it (the two can coexist on the same column).

The engine (Block-Max WAND) is ported from the fulltext_wand branch and lifted into a
standalone pkg/bm25/ package; the index is wired through the existing indexplugin
framework (no per-algo switch in the planner/compiler).

Scope: 112 files, ~22k insertions. 18 commits, organized in phases (see below).

Why

The classic fulltext index stores term positions on every posting to serve
boolean/phrase/NL-with-expansion modes. A large class of retrieval workloads (search
boxes, RAG retrieval, "top-N most relevant docs") only need ranked bag-of-words
top-K
. A position-free BM25 index with Block-Max WAND skips non-competitive documents
and returns the true top-K without scoring the whole postings list — ~5× faster than
the classic fulltext path on the same corpus in our benchmarks, with a much smaller
on-disk index.

Query surface & DDL

CREATE INDEX ftx USING bm25 ON docs(body) WITH PARSER gojieba;

-- ranked top-K; LIMIT pushed into the WAND walk
SELECT id FROM docs WHERE BM25(body) AGAINST('influenza vaccine') LIMIT 10;

-- score in projection
SELECT id, BM25(body) AGAINST('influenza vaccine') AS score FROM docs
WHERE BM25(body) AGAINST('influenza vaccine') ORDER BY score DESC;

-- filtered retrieval (predicate pushed into the walk as a membership bitset)
SELECT id FROM docs WHERE BM25(body) AGAINST('influenza vaccine') AND category=10 LIMIT 10;

ALTER TABLE docs ALTER REINDEX ftx BM25 MERGE;   -- incremental tiered merge
ALTER TABLE docs ALTER REINDEX ftx BM25;         -- full rebuild-from-source

BM25() is a distinct verb from MATCH() — disambiguated by function name, so a
column can carry both a classic fulltext index and a bm25 index: MATCH(col) targets
the former, BM25(col) the latter, with no mode heuristics.

What's in this PR

Engine — pkg/bm25/wand (Phase 0)

Block-Max WAND top-K, two-tier LSM (tag=0 capacity-bounded base subs + tag=1
CdcTail delta/delete frames), membership-bitset prefilter, pk serialize/deserialize.
Ported byte-for-byte from fulltext_wand, relocated and de-fulltext'd.

Index type identity & grammar (Phase 1)

  • catalog.MoIndexBm25Algo (USING bm25), hidden-table type constants.
  • Grammar: USING BM25 [WITH PARSER …], REINDEX … BM25 [MERGE], and the distinct
    BM25(col) AGAINST('q') expression production (goyacc regen, 0 conflicts).

The bm25 AlgoPlugin — pkg/bm25/plugin (Phases 1b–4)

runtime (catalog hooks: 2 hidden tables, AlwaysAsync, SyncDescriptor,
ParamsFromTree) · compile (sync/async create, HandleReindex REBUILD/MERGE,
restore, ValidateReindexParams) · plan (BuildSecondaryIndexDefs, inert
vector hooks) · idxcron (tail-threshold Updatable, ReindexOption) · iscp (CDC
writer). Blank-imported in indexplugin/all + indexplugin/iscp.

Hidden tables & TVFs (Phase 2)

Storage (chunked WAND blobs) + metadata (one row per sub-index); no postings table.
bm25_create, bm25_search, bm25_compact TVFs registered in
pkg/sql/colexec/table_function.

Query routing (Phase 3, then decoupled)

BM25()bm25_match function → bm25_search TVF INNER-JOINed to source on pk,
SORT by score DESC, LIMIT pushdown, optional membership-bitset pre-filter for
… AND <filter>. Lives in apply_indices_bm25.go (self-contained;
apply_indices_fulltext.go untouched), sharing only 3 domain-neutral helpers in
apply_indices_match.go.

Live maintenance (Phase 4)

Post-create DML → tag=1 CdcTail via the ISCP WAND sinker (async). idxcron
cluster-singleton fires scheduled ALTER REINDEX … MERGE. Per-index DAY/HOUR/
SECOND interval + MO_IDXCRON_TICK_SEC bootstrap-tick env (also applied to
ivfflat/cagra/ivfpq for SECOND consistency; hnsw skipped).

Primary-key support

encodePk covers int32/64, uint32/64, varchar/char/text/datalink, uuid,
date/datetime/time/timestamp, decimal64/128, and composite pk (packed varchar).
Unsupported single-column pk types (tinyint/smallint/float/bit/bool/enum) are
rejected at CREATE INDEX, not silently failed at CDC time.

Testing

  • Engine unit (pkg/bm25/wand, 9 files) — build/search/merge/serialize/tail/
    deletes/compaction, incl. a string-pk MERGE regression guard.
  • Plugin unit (pkg/bm25/plugin/{plan,runtime}) — schema build, pk-type
    validation, param extraction, sync descriptor (0% → ~63%/71% coverage).
  • BVTtest/distributed/cases/fulltext/bm25_* (retrieval, datetime, uuid,
    multibase, limit, mode_guard, coexist, pushdown) +
    pessimistic_transaction/bm25/* (basic, async, reindex, merge, clone, restore).
    All pass 100%; pre-existing fulltext_bm25.sql (195 assertions) unchanged.

Self-review (findings addressed)

  • [HIGH] string-pk MERGE panic ([]byte used as an un-normalized map key) → fixed
    with normalizeKey + extracted survivingDeletes helper + regression test.
  • [MED] unsupported pk types silently aborted CDC → now rejected at CREATE.
  • [MED] plugin subpackages had 0% Go coverage → added CPU unit tests.
  • Cross-CN cache staleness — confirmed to be the shared vector-index cache model
    (TTL-bounded), not a bm25 regression.

Index-plugin framework compliance

  • No new per-algo switch/case MoIndex<X>Algo in pkg/sql/{plan,compile} /
    pkg/catalog — routed via indexplugin.Get.
  • No plugin sub-package imports pkg/sql/plan|compile.
  • var _ AlgoPlugin / var _ Hooks assertions intact (7 for bm25).
  • Blank-imported in indexplugin/all + indexplugin/iscp.

Checklist

  • go build / go vet clean on all touched packages.
  • Engine + plugin unit tests green.
  • BVT suite green (fulltext-dir + pessimistic bm25 cases).
  • Self-review gate run; findings fixed or decision-logged.
  • Reviewer sign-off.

cpegeric and others added 30 commits June 4, 2026 13:33
…fault

The stream-ordered pool_memory_resource was set as the global per-device
default (set_per_device_resource). The pool is process-static but CUDA
streams are not: every allocation routed through get_current_device_resource()
— the cuVS index body (ivf_pq/cagra/ivf_flat build) and the non-worker
cuvs::distance::pairwise_distance scratch — was freed back into the pool
tagged with a worker stream that gets destroyed at index drop (worker->stop()).
That left a poisoned free block / sticky cudaErrorInvalidResourceHandle which
aborted the next checked CUDA call (pool do_deallocate -> cudaEventRecord),
surfacing as an intermittent SIGABRT in GPU pairwise scans after build/drop
churn (~1-in-2 full runs under CUDA_LAUNCH_BLOCKING).

Fix: reach the pool only via worker_pool_mr() and pass it explicitly to the
cuvs_worker handle's grow-only search-workspace uvectors (ensure_uvec_), whose
streams are stable and freed before teardown. Everything else uses the plain
default cuda_memory_resource (cudaMalloc/cudaFree), so a dying stream can no
longer poison the pool. Search hot-path perf preserved.

Also: reorder index destroy() to free GPU memory before worker->stop()
(hygiene), and fix cgo/Makefile so libmo.so / libmo.a always re-link from the
freshest cuvs/cuda objects (mo-service loads libmo.so dynamically; a stale one
silently ran old C++) and pin .DEFAULT_GOAL := all.

Validated under both CUDA_LAUNCH_BLOCKING and normal mode: 4-SQL oracle 45/45,
full gpu_cases suite 5/5, zero cudaErrorInvalidResourceHandle aborts.

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

Add SupportedVectorTypes(), SupportedPrimaryKeyTypes(), and
SupportedIncludeColumnTypes() to the index-plugin catalog Hooks (plus helper
predicates), so plan validators and table functions can query what each index
plugin supports instead of hardcoding type lists.

Nil-slice semantics: vector nil = none supported; primary-key nil = any type;
include-column nil = none. Each plugin (cagra / ivfpq / ivfflat / hnsw /
fulltext) declares its supported types in runtime/schema; consumed by
validateIncludeColumns in build_ddl / plugin_builder and by the GPU/CPU
create+search table functions through a shared CatalogHooks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verify the pkg/vectorindex/metric distance types (L2 / L2sq -> L2Expanded,
inner_product -> InnerProduct, cosine -> CosineExpanded) are supported by the
CAGRA and IVF-PQ GPU indexes: each builds, searches, returns the correct
nearest neighbor, and returns a score with the right sign (InnerProduct is
negated on the C++ side to match MatrixOne's inner_product ordering).
Behind the `gpu` build tag.

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

Two bugs found in branch self-review:

1. RunCuvs / runHnsw leaked the sync (GPU/native index resources) when the
   factory/NewSync constructed it inside the txn callback but the wrapping
   transaction commit then failed: the early `if err != nil { return }` was
   placed before the `defer sync.Destroy()`, so the constructed index was never
   released. Destroy it in the error branch (pkg/iscp/cuvs_writer.go,
   index_consumer.go).

2. CagraSearch/IvfpqSearch.buildMultiIndex returned a nil index on an
   unsupported metric, which Load swallowed and Search treated as an (empty)
   success — masking a real misconfiguration. buildMultiIndex now returns
   (idx, error): a bad metric is an error, while the legitimate empty-index case
   still returns (nil, nil). Load propagates it (the existing defer cleans up).
   Updated the 5 search_test.go call sites.

go vet -tags gpu clean for cagra/ivfpq/iscp.

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

From branch self-review (non-bug nits):

- idxcron CuvsUpdatable counts CDC-tail growth straight from the chunk frame
  header (UnframeCdcChunk -> n_inserts + n_upserts), so deriveCuvsRecordShape
  (which parsed the TableDef + resolved INCLUDE columns every tick to produce a
  dim/includeBytesPerRow that countTag1Records discarded) was pure waste. Remove
  it and includedColumnsFromAlgoParams, the orphaned pb/plan import, and the
  unused params; fix the stale countTag1Records doc. The real decode path
  (cdc.go DecodeEventRecord, used by replay/load) is untouched. (+8 / -86)

- cuvs_worker.hpp: add a "DECLARATION ORDER IS LOAD-BEARING" guard comment at the
  search-workspace uvectors — they must be declared after res_ so they're freed
  (into the stream-ordered pool) before the worker stream is destroyed; reordering
  would re-introduce the cudaErrorInvalidResourceHandle pool poisoning. Also
  refresh the now-stale "ensure_rmm_pool_for_device" comments to reference
  worker_pool_mr (it's a no-op post-plan-C) in cuvs_worker.hpp / ivf_pq.hpp.

- cagra/ivfpq create table functions: document that the CDC-cutoff COUNT(*) runs
  via NewSqlProcess(proc) on the same txn/snapshot as the source-row stream, so
  the rowsSeen >= cdcCutoff split can't drift under concurrent writes.

Verified: idxcron + cuvs + iscp unit tests pass; go vet -tags gpu clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In SHARDED mode each rank owns one shard with its own shard_offset, but the
per-shard deleted-bitset cache (gpu_index_base_t::device_shard_bitsets_) was
keyed by physical device_id. Under the gpu_multi_simulation [0,0,...] device
list multiple ranks map to one physical device, so two shards collided on a
single device-0 cache entry; with version-based invalidation the second shard
saw the same bitset_version_ and reused the first shard's slice — dropping that
shard's deletes, so filtered SHARDED search returned soft-deleted rows. Real
multi-GPU is unaffected (device_id is unique per shard there).

Fix: key device_shard_bitsets_ / sync_shard_bitset / acquire_delete_bitset_device
by rank (consistent with replicated_indices_/replicated_datasets_). The full
deleted-bitset cache (device_deleted_bitsets_, REPLICATED/SINGLE) stays
device_id-keyed since replicas share an identical bitset per physical device.

Tests:
- C++ cgo/cuvs/test/ivf_flat_test.cu::SimulatedShardedDeleteSearch
- Go  pkg/cuvs/simulation_test.go::TestSimulatedShardedDeleteIvfFlat
  (build a SHARDED [0,0] index, soft-delete one row per shard, assert excluded;
   cross-shard is guaranteed via the explicit in-order dataset)
- BVT test/distributed/gpu_cases/vector/vector_sharded_filtered.sql
  (filtered SHARDED search under simulation; the per-shard user-filter half)
- BVT pessimistic_transaction/vector/vector_{ivfpq,cagra}_sharded_delete.sql
  (end-to-end CDC/ISCP soft-delete across shards)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…key doc drift

Follow-ups from the SHARDED rekey review (the device_shard_bitsets_ rank fix is
in 32aa19c):

- brute_force build: wrap cuvs::neighbors::brute_force::build in
  device_build_mutex(get_device_id()) for consistency with ivf/cagra's
  per-physical-device serialization of index-mutating GPU calls. brute_force has
  no device-global kmeans workspace, so this is policy consistency rather than a
  known race; nesting it innermost under this->mutex_ is deadlock-free (ivf/cagra
  never hold this->mutex_ while holding device_build_mutex, so no lock-order cycle).

- doc drift: replicated_indices_/replicated_datasets_ are rank-keyed now, but
  several comments still said [dev_id]/[last_dev_id]. Updated them to [rank]
  (index_base.hpp, ivf_flat.hpp, ivf_pq.hpp). The "serialize ... same physical
  device" comments are left as-is — they describe device_build_mutex, which is
  correctly physical-device-keyed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CAGRA/IVF-PQ GPU create path derived the small-tail CDC cutoff from a
plain COUNT(*), but the row loop advanced the build cursor (rowsSeen)
before skipping NULL-vector rows. NULL rows could shrink the final build
chunk below the cuVS minimum graph size.

- fetchSrcTableRowCount now counts only rows with a non-NULL indexed
  vector (WHERE <vec> IS NOT NULL), passed via tblcfg.KeyPart.
- The build cursor advances only after the NULL check, so NULL rows no
  longer move the chunk/cutoff position.
- Shared identically between CAGRA and IVF-PQ.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ALTER TABLE COPY's cloneUnaffectedIndex inferred "skip the whole index"
from SyncDescriptor.UsesCDC, which wrongly caught async IVF-FLAT: its
metadata + centroids were never cloned, so CDC rebuilt entries against an
empty seed k-means model.

- Add AlterTableCloneBehavior.SkipWholeIndex; alter.go reads it instead
  of inferring from UsesCDC.
- HNSW/CAGRA/IVF-PQ/fulltext set it true (they rebuild every hidden table
  via CDC from ts=0); IVF-FLAT leaves it false and uses its
  per-hidden-table policy (delete all three, clone metadata+centroids,
  skip only entries when async).

UsesCDC stays: it still gates CDC task creation / sinker type in
iscp_util.go, which is orthogonal to the clone decision.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Index plugins and the cuVS CDC/idxcron helpers assembled identifiers and
literals by string concat, which breaks (or injects) on a backtick in a
db/table/column name or a single quote in a JSON params blob.

- Add pkg/common/sqlquote: Ident / QualifiedIdent (backtick-quote,
  doubling embedded backticks) and EscapeString / String (single-quote
  escape), with property + injection tests.
- Route the CAGRA/IVF-PQ/HNSW plugin compile builders, the cuVS
  cdc/idxcron/model/build helpers, and the fulltext/IVF-FLAT iscp sinker
  through it.

For ordinary names/literals the generated SQL is byte-identical to before
(proven by Ident/String no-op property tests + golden CROSS APPLY
templates); the helpers only change output for the special-char inputs
that were already producing broken SQL. Identifiers stay column
references (backticks), params/config stay string literals (single
quotes) -- kinds preserved.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
encodeInsertOrUpsert mapped both an absent vector AND a non-nil value of
the wrong type to a DELETE (`if !ok || v == nil`). A type mismatch is a
real schema error, not a NULL vector — silently turning it into a DELETE
drops the row from the index without any signal.

Split the cases (mirrors the HNSW sinker): a nil interface or typed-nil
[]float32 slice still maps to DELETE (actually-absent vector), but a
non-nil wrong-type value now returns an error. Adds a table-driven test
covering all four cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a CPU-reachable BVT that pins the new index-plugin dispatch and
experimental-flag behavior for the GPU-backed algorithms:

- experimental_ivfpq_index / experimental_cagra_index default to 0, are
  settable to 1, and read back.
- With the flag enabled, CREATE INDEX ... USING ivfpq/cagra on a CPU-only
  build is rejected cleanly ("unsupported index type") rather than
  crashing or silently building an empty index.
- Both flags are reset to 0 after use so they don't leak into other cases
  sharing the session.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vector_ivfpq_cagra_cpu asserted CREATE INDEX USING ivfpq/cagra fails with
"unsupported index type" — but that outcome is build-dependent: on a GPU
build the same DDL succeeds. A CPU cases/ case whose .result flips by build
can't be run or regenerated on a GPU box, so it doesn't belong there.

The experimental-flag + plugin-dispatch coverage it was meant to provide is
covered build-independently by vector_index_plugin_smoke and
fulltext_plugin_smoke (HNSW/IVF-FLAT/fulltext), plus the GPU snapshot case.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Build-independent CPU BVTs exercising the new index-plugin framework on the
non-GPU surface:

- vector_ivfpq_cagra_experimental_var: the experimental_ivfpq_index /
  experimental_cagra_index variable surface only (default, SET, SET GLOBAL,
  SHOW VARIABLES) — no index is created, so the result is identical on CPU
  and GPU builds.
- vector_index_plugin_smoke: HNSW (flag gate off->error, on->build) and
  IVF-FLAT dispatch end to end — plugin registration in mo_catalog.mo_indexes,
  SHOW CREATE TABLE round-trip, and a vector search.
- fulltext_plugin_smoke: the fulltext plugin dispatch — registration,
  SHOW CREATE TABLE round-trip, and a MATCH query.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a gpu_cases/snapshot BVT (mirroring cases/snapshot/fulltext_snapshot_
restore.sql) that builds an IVF-PQ / CAGRA index, snapshots, mutates, then
restores the snapshot view and confirms the index survived: row count
restored, index def intact via SHOW CREATE TABLE, and — after a sleep for
the async CDC tail to catch up — the vector search returns the restored
rows (1,2,3).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a gpu_cases/snapshot BVT that exercises the real restore path (not a
data-level re-insert): a sub-account owns ivfpq/cagra indexes, sys snapshots
the account, the account turns the experimental flags OFF and drops its
database, then `restore account ... to account ...` recreates the tables and
replays CREATE INDEX in a background context.

Verifies:
- restore succeeds with the flags off — confirming the experimental gate is
  bypassed on background re-entry (IsFrontend()=false);
- row count + index defs are restored (count=20, SHOW CREATE TABLE);
- both indexes are searchable (1,2,3) after a sleep for the async CDC tail
  to rebuild — restore excludes the index hidden tables (snapshot.go:1945)
  and rebuilds the model from re-inserted main-table data via CDC, so the
  sleep is required by design.

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

Copy link
Copy Markdown
Contributor Author

Can you please break up PRs into pieces that only relates to the intended feature? I don't see why we should include tens of thousands of LOC that is totally unrelated to the feature/fix.

We have to wait for the GPU PR to merge before merge this PR.

cpegeric and others added 2 commits July 13, 2026 08:14
CI patch-coverage gate flagged bm25 files below 0.75 (BVT-covered code doesn't
count toward Go unit coverage). Add CPU unit tests:

- apply_indices_bm25_test.go: findMatchBm25Index, findBm25IndexTables,
  buildBm25SearchTableFunc, and the shared apply_indices_match helpers
  (equalsMatchFunc / findEqualMatchFunc / matchRewriteContextNodeID). Lifts both
  files to ~78-100% per function.
- tree/fulltext_match_test.go: NewBm25MatchFuncExpression + Format (BM25 deparses
  as BM25 with no mode; classic MATCH keeps its mode) + Valid rejects empty
  cols/pattern.
- tokenizer/word_id_test.go: WordID resolves a dict word to a stable id and
  returns ok=false for out-of-dict tokens.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cover bm25_create / bm25_search / bm25_compact Prepare (now 100%) and the start()
config/arg validation branches (bad type, non-const, empty, malformed JSON, bad
capacity, wrong arg count). The main build/search loop stays BVT-covered — it needs
a live WAND index + cache that unit tests can't cheaply stand up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cpegeric and others added 2 commits July 13, 2026 09:16
Resolve conflicts:
- pkg/vectorindex/{cagra,ivfpq}/search_gpu.go: keep cuvs's [B,Q] quantizer
  generics + []B query check; adopt main's ClampSearchLimit/SearchResultPreallocate
  limit-and-allocation hardening (drop the superseded limit:=rt.Limit and []float32
  check).
- pkg/container/vector/vector.go: combine cuvs's narrow-type (bf16/f16/int8/uint8)
  InplaceSortAndCompact cases + helper with main's default:return + SetSorted(true);
  re-add the "sort" import.
- Makefile: combine cuvs's $(GOEXPERIMENT_OPT)/$(GO) with main's $(GO_MODULE_MODE).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Brings main's changes (via cuvs_quantize) into the bm25 branch. One conflict in
apply_indices_fulltext.go: keep the unified findEqualMatchFunc (bm25's shared helper)
and adopt main's fulltext pagination block (paginationLimit/paginationOffset ->
buildCandidateLimit -> limitExpr, with the internalLimit=nil relevance guard when an
explicit sort is present).

Because bm25 and classic fulltext share the unified applyJoinFullTextIndices, main's
pagination applies to bm25_search too — verified: BM25 LIMIT k pushes the limit into
the WAND walk, LIMIT/OFFSET paginates correctly, and BM25 ... ORDER BY <other col>
LIMIT k does NOT push the limit into bm25_search (relevance guard). bm25 BVTs 322/322;
fulltext_bm25 202/202.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cpegeric and others added 10 commits July 13, 2026 10:17
Two changes to the bm25 plugin surface:

1. Parser: bm25 only ever tokenizes with the shared jieba tokenizer (both
   bm25_create and bm25_search hardcode it and never read the parser param), so
   accepting ngram was misleading — a user asking for ngram silently got jieba.
   Restrict supportedParsers to {gojieba, default} (default is a gojieba alias).

2. Experimental gate: bm25 is now feature-gated behind the experimental_bm25_index
   session var (default off), mirroring experimental_hnsw_index. ExperimentalFlag()
   returns Bm25IndexFlag; HandleCreateIndex re-checks it on the frontend path (the
   framework gate in pkg/sql/compile/util.go covers CREATE TABLE ... index). New
   system var in variables.go.

All bm25 BVTs set experimental_bm25_index=1 before CREATE INDEX; new bm25_gate case
asserts the gate (off -> rejected, on -> works) and ngram rejection is covered in the
runtime unit test. bm25 fulltext-dir 341/341, pessimistic 94/94. Benchmark connect()
enables the flag too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts:
#	pkg/sql/colexec/external/parquet.go
# Conflicts:
#	pkg/sql/parsers/dialect/mysql/mysql_sql.go
# Conflicts:
#	pkg/sql/parsers/dialect/mysql/mysql_sql.go
…— port fulltext2's residency wins

Ported the loaded-side residency optimizations from the sibling fulltext2 engine to
bm25's WAND (adapted for bm25's word-id dict + position-free postings). Before, a loaded
BASE segment expanded EVERY posting into resident C buffers, uncompressed (docIDs as raw
int64, 8 bytes/posting) with one *termPostings per term eager — the whole index in RAM.

Four changes:
1. On-disk format: flat uncompressed docIDs/tfs -> three members: wandidx (word-id ->
   ranking byte-offset directory), wandrank (per-term self-contained Block-Max skip
   entries), wandblk (per-BlockSize block: docID gaps delta+varint + raw tfs). ~4×
   smaller and mmap-viewable. (Format is free to break — no version-compat kept.)
2. mmap base load: LoadFromStorage materializes on the LOCAL-SSD fileservice and
   mmapReadOnly's it; blockData/ranking are views into the mapping (reclaimable page
   cache, not C buffers). Free() munmaps + unlinks. New mmap_unix.go/mmap_other.go.
3. Lazy per-term directory: no term decoded at load; decodeTermEntry decodes ONE term's
   Block-Max entry on lookupTerm (via the compact word-id->offset map) — resident
   directory O(query), not O(vocabulary). The WAND cursor now fillBlock-decodes one block
   on demand instead of indexing a full resident array.
4. ordAllowSet []bool -> docBitset []uint64 (1 bit/doc, 8× smaller) for liveness/allow.

Build-side (Builder/Merge/FilterLive/Split) unchanged in behavior — the accessors handle
both build (docIDs!=nil) and loaded (blockData!=nil) reps. Block-Max WAND pruning and
BM25 scoring are identical. bm25 wand unit suite 35/35 (incl. serialize round-trip +
WAND-vs-brute-force differential); bm25 BVT 140/140 live (exercises the mmap load).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CDC tail decodes into the Go heap (frames from the streamed temp file); a huge — or
pathologically-many-tiny-chunk — tail could OOM-kill the CN, taking down every query on
the node, not just the offending one. Before loading, checkTailLoadBudget compares the
tail's ACTUAL stored bytes (SUM(LENGTH(data)) — not the padded span*MaxChunkSize file, so
a pile of 1-row chunks isn't wildly over-counted) against the memory budget:

    avail = system.MemoryTotal()*0.8 - system.MemoryGolang()

MemoryTotal is cgroup-aware; MemoryGolang (live Go heap) already includes tails currently
resident, so this gates an INCREMENTAL load. The mmap'd base is reclaimable page cache and
is deliberately NOT counted (it can't OOM-kill). Over budget -> a clear, actionable error
naming the table, sizes, and fix (ALTER ... REINDEX to compact, or raise CN memory) — a
graceful failure of one query instead of a fatal OOM of the whole CN. The 0.8 headroom
absorbs the (small, post-mmap) C-allocator/query-mpool usage the formula omits.

bm25 wand unit suite green; bm25 BVT 140/140 (guard runs on every tail load, normal loads
unaffected).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… per-iter alloc

The WAND search loop re-sorted the term cursors by current doc on every pivot
iteration via sort.Slice. sort.Slice boxes the []*cursor into an interface{}
(runtime.convTslice) and heap-allocates the less closure on each call; in this
hot per-pivot loop (thousands of iterations/query) that alloc churn dominated:
CPU profile showed 27% runtime.memclrNoHeapPointers (span init for the churn),
37% mallocgc (convTslice 43% + closure newobject 52%), and 36% in the reflect
Swapper sort itself — together ~65% of query CPU spent on the sort, not search.

Cursors are nearly sorted between iterations (only the skipped cursor advances),
so an in-place insertion sort is O(len) here and allocates nothing. Ordering
among equal-curDoc cursors is irrelevant (the pivot-extension loop accounts for
every cursor sitting on pivotDoc), so correctness is unchanged; wand unit tests
pass. Measured on 200K-doc gojieba index: +37% qps (371->510), p50 -27%
(0.77->0.56ms), p95 -27% (11.7->8.6ms), memclr 27%->2.5%.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two hot-path optimizations on the WAND cursor, found by CPU profiling the
200K-doc gojieba index after the sort.Slice fix:

1. Cache curDoc. curDoc() is read 5-10x per pivot iteration (insertion sort,
   pivot scan, blockMax, chooseSkip, alignment) while the cursor moves at most
   once, yet each call re-ran the df() bound check, the pos/BlockSize division
   in ensure(), and a modulo. Cache the current doc in cursor.cur and recompute
   it only on move (refresh, called from newCursor/advance/skipTo). curDoc()
   becomes a field read. All four pos-mutation sites set cur, so the cache never
   goes stale. This alone: +64% qps (504->825), p95 -48% (8.7->4.6ms); curDoc
   fell 15.6%->4% flat and ensure() left the profile.

2. Inline the in-block lower-bound search in skipTo (was sort.Search with a
   per-call closure) — removes the closure alloc and generic frame from the hot
   skip path. Marginal on its own but keeps the skip path allocation-free.

Correctness unchanged (wand unit tests pass); cumulative vs pre-optimization
baseline: 2.2x qps, p95 cut ~60%.

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

@aunjgr aunjgr 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.

The BM25 feature is mixed with hundreds of unrelated cuVS and vector changes, producing a conflicting diff of roughly 384 files and more than 43k additions. The feature cannot be reviewed or merged safely in this shape. Please split or recreate the BM25 work on current main with only its required dependencies.

aunjgr
aunjgr previously requested changes Jul 20, 2026

@aunjgr aunjgr 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.

The BM25 feature is mixed with hundreds of unrelated cuVS and vector changes, producing a conflicting diff of roughly 384 files and more than 43k additions. The feature cannot be reviewed or merged safely in this shape. Please split or recreate the BM25 work on current main with only its required dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature size/XXL Denotes a PR that changes 2000+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants