feat: expose distance-bounded nearest search in C and C++ - #90
Conversation
There was a problem hiding this comment.
✅ Gate recommendation: approve.
This adds C and C++ entry points for Lance's existing half-open distance range. The Rust setter copies and validates bounds before changing the nearest query, and the scanner passes them to Lance for flat and indexed searches while retaining the existing k limit and ANN candidate behavior. The focused range tests and native C/C++ consumers passed.
LuciferYang
left a comment
There was a problem hiding this comment.
Clean addition. I checked the control flow and the docs against the code, plus the underlying Lance distance_range at the pinned rev, and found no correctness or design concerns; the implementation looks good to me. A few non-blocking, LOW test-completeness notes inline. None block merge, so fold them in whenever it suits.
| rows += array.length; | ||
| array.release(&array); | ||
| } | ||
| assert(rows == expected[mode]); |
There was a problem hiding this comment.
test_distance_range in both bindings only asserts the returned row count (test_cpp_api.cpp:1231, test_c_api.c:1343). It never checks that the returned distances fall in [lower, upper) or which ids come back. The core behavior is already pinned in Rust by test_scanner_distance_range_flat_and_indexed_boundaries (ids, in-range distances, boundary inclusivity), so this doesn't affect correctness; the binding tests mainly exercise the C ABI and the C++ wrapper plumbing.
A count-only assertion is still blind to an inclusivity flip: nothing in the fixture sits exactly on 0.125, so flipping the exclusive upper to inclusive wouldn't change the count. A distance-in-range and id check per mode would close that.
There was a problem hiding this comment.
Addressed in 3dc61c4. Both native consumers now check the returned ID set, uniqueness, row count, and every distance against the configured interval. I also added lower=0 and upper=0 modes: the exact self-match at distance zero makes an inclusivity flip observable, which checking the original non-boundary samples alone would not do. All three native consumer tests pass.
| for (lower, upper) in [ | ||
| (f32::NAN, 32.0), | ||
| (0.0, f32::NAN), | ||
| (f32::NEG_INFINITY, 32.0), |
There was a problem hiding this comment.
The non-finite table in test_scanner_distance_range_validation_is_atomic_and_copies_bounds (c_api_test.rs:7457) covers NaN on both sides, -inf as lower, and +inf as upper, but not the mirror cases: +inf as lower and -inf as upper. The current guard is a symmetric !value.is_finite() (scanner.rs:2551) that rejects both directions, so there's no real bug today.
Adding those two rows just completes the matrix. It would catch a future regression where the finite check turned sign-aware.
There was a problem hiding this comment.
Addressed in 3dc61c4. The validation table now tests NaN, +infinity, and -infinity independently as either bound, with the opposite bound unset. This is intentional: pairing +infinity as lower or -infinity as upper with a finite opposite bound could still fail the ordering check and conceal a broken finite-value guard. The test checks INVALID_ARGUMENT and verifies that rejected calls preserve the previous range. All five focused range regressions pass.
| ASSERT(lance_scanner_nearest(scanner, "embedding", query, 8, | ||
| LANCE_DTYPE_FLOAT32, 20) == 0, "nearest failed"); | ||
| ASSERT(lance_scanner_set_use_index(scanner, false) == 0, "use_index failed"); | ||
| ASSERT(lance_scanner_set_distance_range(scanner, mode == 1 ? &lower : NULL, |
There was a problem hiding this comment.
The C-side test_distance_range (test_c_api.c:1313) only runs the success and clear modes. It never calls with an invalid argument: no lower >= upper, non-finite, or missing-nearest case. The C++ side has one reject case at test_cpp_api.cpp:1215.
The -1 return of the extern "C" entry point is already covered by the Rust FFI tests that call lance_scanner_set_distance_range directly, so the reject path isn't missing overall; this is just the .c file's own completeness. Adding one assertion that lower >= upper returns -1 would do it. Non-blocking.
There was a problem hiding this comment.
Addressed in 3dc61c4. The native C test now submits a reversed range and asserts both the -1 return and LANCE_ERR_INVALID_ARGUMENT. It then reads and validates IDs and distances using the previously configured valid range, also checking that rejection did not mutate the configuration. The native C and C++ consumers and static OSS transport test all pass.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
Forwarding validated bounds to Lance remains appropriate for this API: the half-open interval, copied finite values, nearest query's k cap, and approximate index candidate selection are explicit. The revised Rust validation test and both native C/C++ consumers passed, including exact zero-distance boundaries, result IDs and distances, and preservation of the configured range after a rejected call.
Expose Lance's distance bounds through
lance_scanner_set_distance_rangeand the fluent C++Scanner::distance_rangewrapper. For example,nearest("embedding", query, dim, 100).distance_range(std::nullopt, 0.5f)returns at most 100 neighbors whose distance is below 0.5.The interval is lower-inclusive and upper-exclusive. Either side can be unbounded; clearing both removes the range. Bounds are copied, finite, and validated before changing state. Configure them after single-vector nearest and before execution; replacing the query clears them. Multi-vector queries are rejected because their aggregate scores cannot use per-subvector range filtering safely.
This preserves Lance's existing Top-K limit and ANN candidate-selection semantics. It does not add unlimited range enumeration. L2 thresholds use squared Euclidean distance. No dependency changes are needed.
Validation:
cargo test --locked: 449 tests passed.cargo test --locked --test compile_and_run_test -- --ignored --test-threads=1: all 3 native consumer tests passed, including real C/C++ range queries and static OSS transport.cargo clippy --locked --all-targets -- -D warnings: passed.cargo fmt -- --checkandgit diff --check: passed.