perf: validate uniqueness via projected indexed scans (#100)#108
Merged
dcfocus merged 1 commit intoJun 27, 2026
Merged
Conversation
Uniqueness validation previously listed and deserialized the entire dataset (all columns, all history including expired/retired/superseded/ tombstoned rows) into a Vec<ContextRecord> on every add / add_many / upsert / update, then linear-scanned for collisions. That made each write O(N) in store size and incremental/streaming appends O(N^2). Replace the full scan with find_existing_keys(), which issues projected, filtered LsmScanner scans over only the candidate keys: - `id IN (...)` projecting just `id` + `content_type` - `external_id IN (...)` projecting just `external_id` + `content_type` Filters are pushed down to each data source, so the id lookup is index-accelerated when an id scalar index is configured, and neither scan deserializes full records or materializes the whole dataset. Behavior is preserved exactly: within-batch duplicate detection and the existing error messages are unchanged; tombstones remain excluded from collisions (a key whose only surviving row is a tombstone is reusable) while superseded / retired / expired non-tombstone rows still reserve their key. The external_id scan is guarded by a schema check so datasets without that column are unaffected. Caller-supplied external_ids are escaped before going into the IN filter. Tests: id/external_id collisions, within-batch dups, reuse-after-delete, reject-after-supersede, the indexed-scan path, large-store correctness, single-quote escaping, and an ignored append-cost benchmark (store grew ~20x while per-call append cost grew ~4x, not ~20x). Closes lance-format#100
This was referenced Jun 24, 2026
dcfocus
added a commit
that referenced
this pull request
Jun 27, 2026
## Summary Adds a **bulk / batch upsert (insert-or-replace by `external_id`)** API across every layer, closing #102. Previously every upsert path was single-record, and `add_many` *rejects* existing `external_id`s, so there was no way to idempotently upsert a batch in one operation. > **Stacked on #108 (issue #100).** This branch builds on the indexed-validation work; review/merge #108 first. Until then this PR shows both commits — the batch-upsert commit is the last one. The diff will reduce to just #102 once #108 lands. ## Core `ContextStore::upsert_many_by_external_id(records) -> Vec<UpsertResult>` applies insert-or-replace per `external_id` in one logical operation, returning one result per input record (in order) indicating inserted vs. replaced, the resulting id, and the final version. Semantics mirror the single-record `upsert_by_external_id` exactly: - every record must carry a non-empty `external_id`; - duplicate `id`s / `external_id`s **within** the batch are rejected (parity with `add_many`); - validation is **all-or-nothing** — nothing is written if any record is invalid; - caller-supplied supersession fields are ignored (replacement is managed by `external_id`); - an insert whose `external_id` already exists on a non-tombstone but hidden row is rejected, exactly as a single insert is. **Composes with #100:** a batch resolves existing `external_id`s in a *single* scan and validates `id` uniqueness via the indexed path (`find_existing_keys`), instead of full-scanning per record. The whole batch is written in one pass (single version bump where records share a shard). Supersession is resolved within the `external_id`-filtered set, which is correct for every flow that creates supersession through the public API (a successor keeps its predecessor's `external_id`). ## Parity across surfaces | Layer | Addition | |-------|----------| | `lance-context-api` | `ContextStoreApi::upsert_many` + `UpsertRecordsRequest` / `UpsertRecordsResponse` / `UpsertResultDto` | | `lance-context-core` | `upsert_many_by_external_id` + `api_impl` dispatch | | `lance-context` (unified) | dispatch to local/remote | | `lance-context-client` | `RemoteContextStore::upsert_many` + `ContextClient::upsert_records` | | REST server | `PUT /api/v1/contexts/{name}/records/batch` | | Python | `Context.upsert_many(records, key="external_id")` (+ pyo3 binding) | ## Tests - **core**: insert, replace, mixed, idempotent re-application, within-batch duplicate `id`/`external_id`, missing/empty `external_id`, `id` collision with store, empty batch, and the BTree-indexed path. - **REST**: insert+replace batch, empty rejected, missing `external_id` rejected. - **Python**: `python/tests/test_upsert_many.py` (insert/replace/idempotent/mixed/within-batch dup/missing external_id/empty). - **README**: bulk-upsert example added next to `add_many`. ## Checks - `cargo test -p lance-context-core --lib` — 65 passed (1 ignored bench) - `cargo test -p lance-context-server` — 17 passed - `cargo fmt --all -- --check` and `cargo clippy --workspace --all-targets -- -D warnings` — clean - Python: new `test_upsert_many.py` (6) + `test_add_many.py` + `test_external_id.py` pass > Note: ~24 pre-existing Python test failures on `main` (mock `_DummyInner.add()` argument-count drift in the single `add`/`upsert` paths and S3 network tests) are unrelated to this change — this PR only *adds* `upsert_many` and does not touch those paths. ## Acceptance criteria (#102) - [x] Upsert a batch keyed by `external_id` in one call (new inserted, existing replaced/superseded), single version bump where possible - [x] Per-record results indicate inserted vs. replaced and the resulting id - [x] Within-batch duplicate `external_id` handling defined and tested (rejected) - [x] Parity across Python / core / REST / client - [x] Tests cover insert, replace, mixed batches, and idempotent re-application - [x] Composes with #100 so batch upsert does not full-scan per record Closes #102
dcfocus
added a commit
that referenced
this pull request
Jun 27, 2026
) ## Summary Adds a retrieval-quality evaluation harness that measures `search`/`retrieve` quality against a labeled query set and compares quality across dataset versions — closing #98. Cross-version regression eval is the differentiator a stateless vector DB can't offer. This PR is **independent** of #108 (#100) and #109 (#102) — it branches from `main` and can be reviewed/merged on its own. ## Core (`lance-context-core::eval`) - **Eval dataset format** — `EvalQuerySet` / `EvalQuery` / `RelevanceLabel`, referencing records by stable `external_id` with optional graded relevance, loadable from / serializable to **JSONL**. - **Metrics** — recall@k, precision@k, MRR, nDCG@k (linear gain), hit-rate, as pure functions. - **Runner** — `ContextStore::evaluate(query_set, config)` scores a set at the current version for **vector** (`search`) or **hybrid** (`retrieve`) mode, with configurable `k`, filters, and lifecycle options. Returns per-query + aggregate scores and a manifest (query-set id, version, k, mode, distance metric). - **Version A/B** — `ContextStore::evaluate_versions(..., baseline, candidate)` checks out each version, evaluates, restores the original version, and reports per-metric deltas. `MetricScores::delta` also enables config A/B (two `evaluate` runs). ## Python - `Context.evaluate(queries, *, k, mode, filters, include_expired, include_retired)` → report dict. - `Context.evaluate_versions(queries, baseline_version, candidate_version, ...)` → `{baseline, candidate, deltas}`. Query sets and reports are marshaled as JSON across the pyo3 boundary, reusing the serde-derived core types. ## Tests - **core** (12): metric correctness vs hand-computed fixtures (perfect / rank-2 / none / graded nDCG / precision@k), JSONL round-trip, vector + hybrid runners, **filter** and **lifecycle** handling, and the version-A/B mechanism (zero-delta + version restore). - **python** (5): `test_eval.py` — vector mode, graded nDCG, config A/B delta, version A/B, invalid-mode error. README gains an eval + version-A/B example. ## Checks - `cargo test -p lance-context-core --lib` — 59 passed - `cargo fmt --all -- --check` and `cargo clippy --workspace --all-targets -- -D warnings` — clean - `ruff format --check`, `ruff check`, `pyright` — clean - `test_eval.py` — 5 passed ## Acceptance criteria (#98) - [x] Run a labeled query set → recall@k / MRR / nDCG for vector and hybrid retrieval - [x] A/B two versions (or two configs) → per-metric deltas - [x] Results pinned to a context version + config in a reproducible report - [x] Tests cover metric correctness against fixtures, filter/lifecycle handling, and the version-A/B mechanism ## Notes - Cross-version A/B with *differing data* is not unit-tested because MemWAL-backed writes don't reliably advance the base-table manifest version (the same limitation `test_time_travel_checkout` `xfail`s on). The A/B **mechanism** (checkout both versions, restore, per-metric deltas) and metric-difference detection are covered via the same-version and config-A/B tests. - The eval-set format is intentionally simple JSONL so it can be shared with the post-training pipeline's eval/holdout set (#96 / #103), per the issue notes. - nDCG uses linear gain (`grade / log2(rank+1)`); documented on `MetricScores`. Closes #98
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.
Summary
Fixes the O(N)-per-write (O(N²) for incremental/streaming appends) uniqueness validation described in #100.
Previously
validate_unique_ids/validate_new_record_idcalledlist_with_options(None, None, LifecycleQueryOptions::new(true, true)), which listed and deserialized the entire dataset — every column, all history (expired/retired/superseded/tombstoned included) — into aVec<ContextRecord>, then linear-scanned forid/external_idcollisions. This ran on everyadd/add_many/upsert/update.This PR replaces the full scan with
find_existing_keys(), which issues projected, filteredLsmScannerscans over only the candidate keys:id IN (...)projecting justid+content_typeexternal_id IN (...)projecting justexternal_id+content_typeFilters are pushed down to each data source, so:
idlookup is index-accelerated when anidscalar index (id_idx) is configured, andPer the issue,
external_id(which has no dedicated index today) is validated via a projected filtered scan rather than a full deserialize. A dedicatedexternal_idscalar index remains a natural follow-up for fully-indexedexternal_idvalidation.Behavior preserved
I first locked in the actual current semantics with a throwaway probe test before refactoring (the issue note slightly overstates the tombstone case —
is_visiblealready filters tombstones even with both lifecycle flags on):external_idscan is guarded by a schema check, so datasets without that column are unaffected.external_ids are single-quote-escaped before entering theINfilter.Tests
New
lance-context-coreunit tests:add_rejects_duplicate_id_against_existing,add_rejects_duplicate_id_within_batch,add_rejects_duplicate_external_id_within_batchadd_allows_external_id_reuse_after_delete,add_allows_id_reuse_after_deleteadd_rejects_external_id_after_supersedevalidate_uniqueness_with_btree_index(indexed-lookup path)validate_uniqueness_against_large_storevalidation_handles_external_id_with_single_quoteappend_cost_does_not_grow_linearly(#[ignore]benchmark)The benchmark grows the store ~20x and asserts per-call append cost stays roughly constant; observed ratio ≈ 3.8x (vs. ~20x+ for the old O(N) path):
Run it with
cargo test -p lance-context-core -- --ignored append_cost.Checks
cargo test -p lance-context-core --lib— 56 passedcargo fmt --all -- --check— cleancargo clippy --workspace --all-targets -- -D warnings— cleanAcceptance criteria (#100)
add/add_many/upsert/updatevalidate uniqueness without listing + deserializing the entire datasetCloses #100