Skip to content

fix(mem_wal): honor distance_range in the LSM vector planner - #8845

Open
hamersaw wants to merge 1 commit into
lance-format:mainfrom
hamersaw:feat/mem-wal-filtered-ranked-search
Open

fix(mem_wal): honor distance_range in the LSM vector planner#8845
hamersaw wants to merge 1 commit into
lance-format:mainfrom
hamersaw:feat/mem-wal-filtered-ranked-search

Conversation

@hamersaw

@hamersaw hamersaw commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

LsmVectorSearchPlanner::with_distance_range threads a [lower, upper) bound into all three source arms, alongside the existing with_filter.

Without it a caller's distance_range was dropped for the fresh tier while the base table honored it, so an out-of-range row could survive a downstream top-k cut — a caller merging the two tiers saw them disagree.

The lower-bound case

A lower bound additionally forces the active memtable to brute force. VectorIndexExec can only apply the bound after its HNSW search has already cut to k, and a lower bound excludes the nearest rows — so top-k-then-filter under-fills or returns nothing. MemTableBruteForceVectorExec filters the complete candidate set before its cut.

An upper bound stays on the HNSW path: it trims the far tail the top-k would have dropped anyway.

Test

test_vector_search_distance_range_bounds_the_search covers both bounds and pins the routing guard. With hnsw_safe_with_bounds removed, the lower-bound probe returns [] instead of [3, 4] — verified, not assumed.

cargo test -p lance --lib mem_wal — 670 passed, 0 failed.

Why

Found while fixing a wrong-answer bug in LanceDB's WAL, where a bounded vector search returned fresh-tier rows outside the bound while the identical base-table rows were filtered correctly. The consuming change is separate; this stands on its own.

🤖 Generated with Claude Code

https://claude.ai/code/session_019czxRomV95czdrXg5hGJZ1

@github-actions github-actions Bot added the enhancement New feature or request label Aug 28, 2026
@hamersaw
hamersaw marked this pull request as ready for review August 28, 2026 16:27

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Gate recommendation: approve with a non-blocking risk.

The distance-range propagation fixes the wrong-answer path at the right boundary: every source arm applies the range before its top-k, and lower bounds avoid HNSW top-k-then-filter underfill.

The residual risk is the new filter-resolution API. Scanner::resolved_filter_expr substantially overlaps the existing public get_expr_filter path but has no focused coverage, leaving two resolution paths that may diverge for metadata-dependent filters. Reusing one implementation and covering no-filter, SQL, and metadata-column cases would reduce that risk.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Aug 28, 2026
@hamersaw
hamersaw force-pushed the feat/mem-wal-filtered-ranked-search branch from 7c23191 to 95cb4ac Compare August 28, 2026 18:11
@github-actions github-actions Bot added the A-index Vector index, linalg, tokenizer label Aug 28, 2026
@hamersaw hamersaw changed the title feat(mem_wal): honor distance_range and expose the resolved filter fix: honor distance_range in the LSM planner; make to_expr fallible Aug 28, 2026
@github-actions github-actions Bot added the bug Something isn't working label Aug 28, 2026
@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Aug 28, 2026
`LsmVectorSearchPlanner::with_distance_range` threads a `[lower, upper)`
bound into all three source arms, alongside the existing `with_filter`.
Without it a caller's `distance_range` was dropped for the fresh tier
while the base table honored it, so an out-of-range row could survive a
downstream top-k cut — a caller merging the two tiers saw them disagree.

A lower bound additionally forces the active memtable to brute force.
`VectorIndexExec` can only apply the bound after its HNSW search has cut
to k, and a lower bound excludes the *nearest* rows — so
top-k-then-filter under-fills, or returns nothing.
`MemTableBruteForceVectorExec` filters the complete candidate set before
its cut. An upper bound stays on the HNSW path: it trims the far tail the
top-k would drop anyway.

`test_vector_search_distance_range_bounds_the_search` covers both bounds
and pins the routing guard: with the guard removed the lower-bound probe
returns `[]` instead of `[3, 4]`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019czxRomV95czdrXg5hGJZ1
@hamersaw
hamersaw force-pushed the feat/mem-wal-filtered-ranked-search branch from 95cb4ac to e5c2540 Compare August 28, 2026 18:22
@hamersaw hamersaw changed the title fix: honor distance_range in the LSM planner; make to_expr fallible fix(mem_wal): honor distance_range in the LSM vector planner Aug 28, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gate recommendation: approve.

The revised scope removes the duplicate filter-resolution API from the prior revision. The remaining change applies [lower, upper) within every LSM source before its top-k and routes lower bounds to brute force, so excluded nearest rows cannot consume the candidate budget. This preserves the intended range semantics while keeping upper-bound searches on the HNSW path.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer bug Something isn't working enhancement New feature or request K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants