Repository navigation
feat: add synchronous index prewarm C and C++ APIs - #94
Conversation
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The thin binding preserves the SDK’s snapshot, error, and shared-session cache semantics. Local regression and C/C++ runtime tests confirm multi-segment prewarm and cache reuse after reopening handles, with correct query results and zero partition loads. No blocking issues found.
| let version = lance_dataset_version(ds); | ||
| let historical = lance_dataset_open(c_str(&uri).as_ptr(), ptr::null(), version); | ||
| assert!(!historical.is_null()); | ||
| assert_eq!(lance_dataset_prewarm_index(ds, c"key_idx".as_ptr()), 0); |
There was a problem hiding this comment.
[P2] Assert the scalar index was actually warmed
This test currently proves that the B-tree call succeeds for a multi-segment snapshot, but not that either segment was loaded into the session cache. Could we also query both segments through the same session and assert the relevant cache-hit/load statistics? That would prevent a no-op or existence-only implementation from satisfying the test.
…ard (branch-4.1) (#68698) ### What problem does this PR solve? Related issue: #68692 (M1) Related PRs: lance-format/lance-c#94, #68687 Upgrade lance-c from `cd63420bfbe27f6f0a1edcc873b9191af7d52852` to `98468344bc9d56aa7ed4a4192e844afa62b34ba1` to include the upstream synchronous index-prewarm C/C++ APIs. Refresh the retained Foyer patch without changing its added/removed implementation payload, and verify that patch application preserves the prewarm APIs and shared-session regression. Remove the Lance-specific installation fingerprint mechanism introduced by #68687: delete `lance-install.sh` and its test harness, remove the CI harness step, and restore the existing dependency build flow. An existing installation is no longer rejected or rebuilt because of a Lance revision fingerprint; missing APIs are reported by the normal compilation or linking process. This delivers the dependency portion of M1. SQL parsing, BE RPCs, and shared-session integration remain outside this PR; the parent issue stays open. ### Validation After rebasing onto the current `branch-4.1`: - Lance archive checksum verified; the Foyer patch's added/removed payload matches the target branch. - `lance-prefilter-patch-test.sh`: passed fresh extraction, cached-source reuse, re-extraction, stale-marker refresh, invalid-patch rejection, and prewarm API/regression preservation checks. - `download-thirdparty-fallback-test.sh` and `adbc-jni-config-test.sh`: passed. - Affected shell syntax, workflow YAML parsing, and `git diff --check`: passed. - Confirmed that the removed installation scripts and fingerprint symbols have no remaining references, and the build/CI scripts match their versions before the fingerprint mechanism was introduced. No FE/BE source files change. Full Doris compilation and runtime suites were not rerun locally and remain for PR CI. ### Release note Prepare the Lance dependency for index prewarm and remove Lance-specific installation fingerprint checks. No new SQL interface is exposed. ### Check List (For Author) - Test: - [x] Existing dependency/script regression tests - [x] Manual validation described above - Behavior changed: - [x] Yes. Upgrade lance-c and restore the existing dependency build flow without Lance revision fingerprint checks. - Does this need documentation? - [x] No new SQL interface; upstream API documentation is included in the dependency. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
…ard (branch-4.1) (#68698) ### What problem does this PR solve? Related issue: #68692 (M1) Related PRs: lance-format/lance-c#94, #68687 Upgrade lance-c from `cd63420bfbe27f6f0a1edcc873b9191af7d52852` to `98468344bc9d56aa7ed4a4192e844afa62b34ba1` to include the upstream synchronous index-prewarm C/C++ APIs. Refresh the retained Foyer patch without changing its added/removed implementation payload, and verify that patch application preserves the prewarm APIs and shared-session regression. Remove the Lance-specific installation fingerprint mechanism introduced by #68687: delete `lance-install.sh` and its test harness, remove the CI harness step, and restore the existing dependency build flow. An existing installation is no longer rejected or rebuilt because of a Lance revision fingerprint; missing APIs are reported by the normal compilation or linking process. This delivers the dependency portion of M1. SQL parsing, BE RPCs, and shared-session integration remain outside this PR; the parent issue stays open. ### Validation After rebasing onto the current `branch-4.1`: - Lance archive checksum verified; the Foyer patch's added/removed payload matches the target branch. - `lance-prefilter-patch-test.sh`: passed fresh extraction, cached-source reuse, re-extraction, stale-marker refresh, invalid-patch rejection, and prewarm API/regression preservation checks. - `download-thirdparty-fallback-test.sh` and `adbc-jni-config-test.sh`: passed. - Affected shell syntax, workflow YAML parsing, and `git diff --check`: passed. - Confirmed that the removed installation scripts and fingerprint symbols have no remaining references, and the build/CI scripts match their versions before the fingerprint mechanism was introduced. No FE/BE source files change. Full Doris compilation and runtime suites were not rerun locally and remain for PR CI. ### Release note Prepare the Lance dependency for index prewarm and remove Lance-specific installation fingerprint checks. No new SQL interface is exposed. ### Check List (For Author) - Test: - [x] Existing dependency/script regression tests - [x] Manual validation described above - Behavior changed: - [x] Yes. Upgrade lance-c and restore the existing dependency build flow without Lance revision fingerprint checks. - Does this need documentation? - [x] No new SQL interface; upstream API documentation is included in the dependency. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
`lance_dataset_prewarm_index()` can fail for a valid vector index when its name matches another schema column. The C++ `Dataset::prewarm_index()` wrapper consequently throws `lance::Error`. Resolve the indexed field from segment metadata for this collision so prewarming succeeds on a fresh session and subsequent queries reuse the warmed partitions. ### Trigger and impact A concrete reproducer is: 1. Create a dataset with `id: Int32` and `embedding: FixedSizeList<Float32, 8>`. 2. Build an `IVF_FLAT` index on **`embedding`**, but name the index **`id`**. The regression uses two fragments and two index segments. 3. Open the dataset with a fresh session, before any vector query has warmed that session. 4. Call `lance_dataset_prewarm_index(dataset, "id")`. Before this fix, the call returns `-1` with an error containing `Data type is not a vector ... but Int32`. Index construction and nearest-neighbor queries on `embedding` succeed for the same dataset. Consequently, callers cannot complete an explicit warmup before serving queries; applications that continue after the error still incur cold index reads. Applications that require warmup to succeed can reject an otherwise usable dataset. The demonstrated failure is in prewarm, without changing persisted data or index contents. The name collision matters: an index name that is not a schema column already resolves through index metadata. The test uses a separate session for the cold control query, so that query cannot populate the session used to reproduce prewarm. ### Cause and fix Reproduced against lance-c `2089f6f` (current `main`), which pins Lance SDK `68c12df`. This is a correctness follow-up to #94. The pinned Lance SDK passes the logical index name to `open_generic_index()`. Its vector field resolver checks for a schema column with that name before consulting the segment's field ID, so `id` incorrectly selects the scalar column instead of `embedding`. For a name that matches a different schema field, the binding loads the named index's segment metadata and opens each affected vector segment using the indexed field path and segment UUID. It prewarms that opened index on the same dataset snapshot and session. The complete open-and-prewarm operation for each affected segment runs concurrently via `try_join_all`, preserving the SDK’s fail-fast error propagation and cancellation of pending segment futures. Other paths continue through the SDK, including missing-index errors and scalar/full-text prewarm with its aggregate residency validation. This keeps the existing C/C++ API and dependency revision. It does not include the Doris-specific Foyer patch. Found while integrating upstream prewarm into [apache/doris#69277](apache/doris#69277 (comment)). ### Validation Linux, Rust 1.97.0, using the unchanged lockfile and default features: - Before the fix: the new cold-session collision test failed with `-1` and the `Int32`/vector type error, after its independent cold-query control succeeded. - `cargo test --locked --test c_api_test test_prewarm_index`: all 6 prewarm tests passed. The vector cases cover non-colliding names, names matching the indexed field, and names matching a different scalar field. Both segments return correct nearest neighbors with **zero partition reloads** after prewarm and a shared-session reopen. - `cargo test --locked --lib index::tests::`: all 3 scheduling tests passed. Deterministic gates verify that all 8 segment opens start before any may finish, and all 8 prewarms start before any may finish. Separate checks verify immediate error propagation/cancellation during open or prewarm, and scalar/unresolved-segment fallback to the SDK. The concurrency tests failed against the serial implementation (`1` segment started instead of `8`); no timing thresholds or sleeps are used. - Coverage also includes B-tree/FTS name collisions, invalid arguments, missing indices (including names matching an existing column), pinned snapshots, repeated calls, and dataset/session handle lifetimes. - `cargo test --locked`: 492 passed, 0 failed; the 3 ignored C/C++ cases were run separately. Doc-tests also completed successfully. - `cargo test --locked --test compile_and_run_test -- --ignored --test-threads=1`: all 3 C/C++ compilation/runtime cases passed. - `cargo clippy --locked --all-targets -- -D warnings`: passed. - `cargo fmt -- --check` and `git diff --check`: passed.
C and C++ callers currently cannot invoke the Rust SDK's index prewarm operation before serving queries. Add
lance_dataset_prewarm_index(dataset, index_name)andDataset::prewarm_index(index_name)as synchronous bindings to the existing SDK method.The operation uses one dataset snapshot and its existing session, prewarms all segments of the named logical index, and preserves the dataset version. It uses the established FFI error/panic handling and rejects NULL, empty, and invalid UTF-8 arguments. The README and public header explain shared-session reuse, eviction, partial failures, snapshot scope, and the distinction between index prewarming and result-row reads. No Lance Rust or dependency changes are required.
Tests cover invalid inputs, missing indexes, multi-segment B-tree and IVF-Flat indexes, historical snapshots after index removal, repeated calls, and dataset/session handle lifetimes. An independent cold query records partition loads; queries from a newly opened handle sharing the prewarmed session return the expected nearest neighbors with zero partition loads. C and C++ runtime tests exercise the exported symbol and wrapper.
Validation:
cargo fmt -- --checkcargo check --all-targetscargo clippy --all-targets -- -D warningscargo test— 479 passed, 0 failed (3 separately executed tests ignored by default)cargo test --test compile_and_run_test -- --ignored --test-threads=1— 3 passed, 0 failedImplements the upstream binding portion of M1 in apache/doris#68692. The Doris dependency update and synchronous SQL integration remain separate follow-up work after this merges.