Merged PR 2194508: Fix bson_aggregation_pipeline_tests vector test br… · documentdb/documentdb@f97c83b · GitHub
Skip to content

Commit f97c83b

Browse files
Merged PR 2194508: Fix bson_aggregation_pipeline_tests vector test broken by pgvector 0.8.5
## Why The `numLists > data size` IVFFLAT block in `bson_aggregation_pipeline_tests` hardcoded a **pgvector build-memory pre-flight estimate** (`ERROR: memory required is 394 MB, maintenance_work_mem is 64 MB`) in its expected output. That number is a pgvector implementation detail, not documentdb behavior. PGDG shipped **pgvector 0.8.5** (commit `b383e4d1`), which changed IVFFLAT sampling to reduce build memory for small tables. The estimate dropped **394 MB -> 383 MB**, and the test started failing at `make check` in the Debian packaging path: ``` expected: memory required is 394 MB actual: memory required is 383 MB ``` Crucially, the two CI paths pin **different** pgvector versions: - regress source build -> pgvector **0.8.1** (394 MB) - deb / PGDG -> pgvector **0.8.5** (383 MB) So hardcoding any single MB number cannot satisfy both paths, and pinning a pgvector version is brittle. The fix has to be **version independent**. The block also used `numLists: 10000` against only **two** vector rows. That always exceeded `maintenance_work_mem` and asserted the memory-rejection *failure* path, so the test never actually exercised the "successful randomized-centroid search" path its own comment describes. ## How Change `numLists` **10000 -> 3** on one SQL line. `3` is the smallest value still greater than the two-row data size, so it preserves the block's stated intent (`numLists > data size`) while letting the index build cheaply. With the index building, the volatile `memory required is N MB` assertion disappears entirely and both searches return real, deterministic results. - **Only one SQL line changes.** The expected `.out` is regenerated because the now-building index turns the former error lines into real result rows and EXPLAIN plans. - `nProbes` values are **intentionally unchanged**: query1 `nProbes:10000` auto-clamps to `numLists`, and query2 `nProbes:1` is deterministic because its query vector exactly equals a stored vector (score `1.0`). Both were proven run-to-run stable and byte-identical across pgvector versions. ## Validation - Full `make check` passes on pgvector **0.8.1** and **0.8.5** (`# All 100 tests passed.`). - Cross-version expected output is **byte-identical** (0 diff), verified with 10x determinism loops per version. - documentdb OSS fork CI green across **PG15-18** on **arm64 and amd64** (sql regress + functional deb `make check`). ## Scope / sync note This edits the OSS test under `oss/pg_documentdb/...`, which syncs to the public `documentdb/documentdb` repo. No separate public PR is needed. The distributed variant (`oss/internal/pg_documentdb_distributed/...`) has no IVFFLAT `numLists` block and is unaffected. Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com>
1 parent 191f906 commit f97c83b

2 files changed

Lines changed: 76 additions & 12 deletions

File tree

‎pg_documentdb/src/test/regress/expected/bson_aggregation_pipeline_tests.out‎

Lines changed: 75 additions & 11 deletions

‎pg_documentdb/src/test/regress/sql/bson_aggregation_pipeline_tests.sql‎

Lines changed: 1 addition & 1 deletion

0 commit comments

Comments
 (0)