Skip to content

feat: expose independent batch nearest search in C and C++ - #92

Merged
yanghua merged 2 commits into
lance-format:mainfrom
Gabriel39:dev/scanner-batch-nearest
Sep 30, 2026
Merged

yanghua merged 2 commits into
lance-format:mainfrom
Gabriel39:dev/scanner-batch-nearest

Conversation

@Gabriel39

@Gabriel39 Gabriel39 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Expose independent batch nearest-neighbor search through lance_scanner_nearest_batch and the C++ Scanner::nearest_batch overloads. A row-major matrix of N vectors produces up to K neighbors per query, identified by a zero-based query_index column, including a batch of one. This provides the binding needed for consumers such as apache/doris#68394.

The existing fixed-size-list query representation also describes multi-vector queries. Track the query mode explicitly so independent batches use Lance's native batch execution instead of the custom multi-vector scoring/window path. Single-vector, multi-vector, and batch queries share input-alignment validation before decoding typed buffers. No dependency changes are needed.

  • Copy and validate input before replacing the configured query. Support Float16/32/64, Int8, and UInt8 (Hamming), require matching column shape/type and finite floating-point values, and reject the reserved query_index dataset column.
  • Distinguish arithmetic overflow from byte-limit rejection, reporting dimension, query count, element width, and the computed size when representable. Unsupported decoded types return an input error.
  • Bound request size and candidate expansion: at most 128 queries, 64 MiB of query values, and num_queries * k * refine_factor <= 100000 (unset refinement counts as 1). These bounds do not cap total index/payload memory.
  • Share scanner filter, metric, index and tuning options across queries. Reject FTS and global scanner limit/offset in either configuration order; consumers must apply result windows separately per query. Distance ranges remain single-query-only; replacing a single query with a batch clears its bounds.
  • Reuse Arrow stream ownership and cancellation. Native shared scans remain conditional on Lance's execution-plan eligibility; this API makes no speedup guarantee.

Validation:

  • cargo test: 469 Rust tests passed.
  • cargo test --test compile_and_run_test -- --ignored: all 3 native consumer tests passed (C, C++, and static OSS transport); batch checks validate per-query ids, distances, counts, and stream ownership.
  • cargo clippy --locked --all-targets -- -D warnings, cargo check --locked --all-targets, formatting, and diff checks passed.
  • All 15 batch integration tests passed, including exact per-query IDs/distances for individually selected IVF_FLAT segments, Int8 L2 queries, and byte-limit diagnostics preserving the previous query.
  • The internal regression first failed on the previous matrix-based mode detection, then passed with explicit query modes. Coverage includes flat/IVF/refined plans, multiple fragments, fragment scope, projection/filtering/empty results, duplicate queries, all supported element types, arithmetic overflow and byte-limit boundaries, input alignment, invalid input preserving state, query replacement, and early stream release.

lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026
@Gabriel39
Gabriel39 force-pushed the dev/scanner-batch-nearest branch from e0fd094 to 26608ca Compare September 30, 2026 10:42
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026

@LuciferYang LuciferYang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the batch nearest-neighbor change. No blocking issues; the validation is solid. Mutual exclusion with limit/offset and FTS is enforced symmetrically in both configuration orders, the error path leaves no half-updated state (the only write to s.nearest is the last statement), and the C/C++ signatures match the Rust FFI.

Four inline notes below: one MEDIUM on error-message quality, and three LOW (a latent unreachable!(), the Int8 API asymmetry, and a thin unit test). I also chased down two things that looked off and cleared both. Batch does not pin a default metric the way multi-vector does, but Lance's planner sets the metric before the TopK merge and single-vector search uses the same wrapper path, so batch cannot diverge. And the README's "UInt8 uses the Hamming metric" line is accurate: default_distance_type_for maps UInt8 to Hamming.

Comment thread src/scanner.rs Outdated
})
.ok_or_else(|| {
invalid(format!(
"batch query values exceed {MAX_BATCH_QUERY_BYTES} bytes"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM · robustness

The byte-cap error "batch query values exceed 67108864 bytes" collapses two distinct failures into one message: dimension * num_queries * width overflowing, and the size genuinely exceeding 64 MiB. It also names none of dimension, num_queries, or the computed byte size. A caller rejected with a legitimate 80 MB request just sees "exceeds 64 MiB" with no hint of which parameter blew the budget; on a 32-bit target the overflow path reports the wrong reason entirely.

AGENTS.md asks for variable names and values in error messages. Splitting the two cases and threading dimension/num_queries/the byte size into each message is enough. Correctness is fine; this is purely to make diagnosis easier.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4ceed34. Checked multiplication now reports usize overflow separately from the 64 MiB limit. Both errors include dimension, num_queries, and element_width; the limit error also includes the computed byte size. Tests cover both multiplication overflow points, the exact byte-limit boundary, and an 80 MB request rejected before reading its buffer while preserving the previously configured query.

Comment thread src/scanner.rs Outdated
.iter()
.all(|v| v.is_some_and(f64::is_finite)),
DataType::UInt8 => true,
_ => unreachable!(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW · maintainability

The finite-check match falls through to unreachable!(). It is genuinely unreachable today only because the element_type match at line 3062 admits 0..=3. But decode_query_values already decodes element_type=4 (Int8), so the two matches are one edit apart: add an Int8 arm at 3062 and decode produces an Int8Array that panics here.

panic = "unwind" turns that into LANCE_ERR_PANIC rather than UB, but it was meant to be a clean invalid_input. Returning Err(invalid(...)) from the fallthrough, or adding an explicit Int8 arm, is safer than leaving unreachable!().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4ceed34. Int8 now has an explicit finite-check arm alongside UInt8, and the fallback returns invalid_input with the decoded type and element_type instead of unreachable!(). Future decoder extensions therefore cannot turn this validation path into a panic.

Comment thread src/scanner.rs Outdated
3 => (DataType::UInt8, 1),
_ => {
return Err(invalid(format!(
"batch element_type ({element_type}) must be float16, float32, float64, or uint8"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW · API consistency

Batch accepts only element_type 0..=3 and rejects Int8 (4), while the single-vector nearest path decodes Int8 via decode_query_values. So the same Int8 vectors work through single-vector search but not through batch, and a caller migrating from one to the other hits a wall.

The README lists only float16/32/64/uint8 for batch, so this reads as intentional. Worth confirming the direction: if Int8 vector search is meant to be supported, add it to batch; if it is not, the single-vector path should reject it too (or the docs should say so), rather than leaving the two APIs with different type sets. (The asymmetry runs both ways: batch adds a query_data alignment check that nearest/nearest_multivector lack.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4ceed34. The pinned Lance dependency supports ordinary Int8 vector queries, so batch now accepts Int8 too. The new L2 regression uses negative Int8 coordinates and checks complete per-query IDs and distances. README and C API documentation list Int8. Alignment validation now lives in the shared decoder used by single-vector, multi-vector, and batch queries and uses each type's actual alignment requirement; a regression covers misaligned floating-point buffers.

Comment thread src/scanner.rs Outdated
counts[*q as usize] += 1;
}
}
assert_eq!(counts, [2, 2]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW · testing

This test asserts only the per-query_index row counts [2, 2], and only on a flat scan (use_index=false); it checks neither distances nor which rows came back. An implementation with correct counts but wrong contents would pass it. The feature as a whole is held up by the full (query_index, id, _distance) comparison against a brute-force expected() in batch_nearest_test.rs, so this is not a coverage gap, just a weak individual test.

Adding a distance/row-membership assertion here would make it carry its weight. Separately, batch combined with index_segments (distributed segment-scoped search) has no test at all, even though build_scanner applies with_index_segments for batch too and the README promises index selection applies; a case for that is worth adding.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4ceed34. The unit regression now asserts complete (query_index, id, _distance) tuples. A new integration test builds two IVF_FLAT segments and tests selecting the first segment, the second segment, and both, comparing each query's IDs and distances against a brute-force result restricted to the selected rows. All 469 Rust tests and all 3 native C/C++ consumer tests passed, along with formatting, cargo check, and Clippy.

@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Gate recommendation: approve.

Native Lance execution provides up to k results per query, tagged with query_index. Copied inputs and explicit query modes preserve ownership and separation from multi-vector scoring. The documented request bounds and conditional shared execution fit this contract; global limit/offset remain rejected.

Int8 uses the existing native vector path. Shared alignment validation rejects misaligned buffers before decoding, and batch size failures preserve the previous query. The expanded coverage checks complete IDs and distances across separately selected IVF segments.

Distance ranges remain a single-vector setting: replacing that query with a batch clears its bounds, and attempts to set batch bounds fail without altering the query.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026

@yanghua yanghua left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@yanghua
yanghua merged commit e69b4af into lance-format:main Sep 30, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants