Skip to content

feat: add configurable FTS index prewarm options#6298

Merged
BubbleCal merged 6 commits into
mainfrom
yang/fts-prewarmwithposition
Apr 9, 2026
Merged

feat: add configurable FTS index prewarm options#6298
BubbleCal merged 6 commits into
mainfrom
yang/fts-prewarmwithposition

Conversation

@BubbleCal
Copy link
Copy Markdown
Contributor

@BubbleCal BubbleCal commented Mar 26, 2026

Summary

  • add PrewarmOptions and FtsPrewarmOptions on the Rust side, with dataset plumbing for prewarm_index_with_options
  • add Python prewarm_index(..., *, with_position=False) support for FTS indices while keeping the default prewarm path unchanged

@github-actions
Copy link
Copy Markdown
Contributor

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

@github-actions github-actions Bot added the A-python Python bindings label Mar 26, 2026
@BubbleCal BubbleCal changed the title Add configurable FTS index prewarm options feat: add configurable FTS index prewarm options Mar 26, 2026
@github-actions github-actions Bot added the enhancement New feature or request label Mar 26, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Code Review

Overall this is a well-structured PR. The approach of separating position data into a dedicated cache during prewarm — so phrase queries avoid a lazy re-read — is sound. The cache flow is correct: take_positions() strips positions from the posting list before caching it, stores them under PositionKey, and read_positions() uses get_or_insert_with_key which naturally hits that pre-populated entry.

Test coverage is solid: both Rust and Python tests verify the cache-entry-count invariant (no growth after prewarmed phrase query), the validation error for no-position indices, and the Python keyword-only argument enforcement.

Minor observations (non-blocking)

  1. Stale commentindex.rs:1674 still says "when the cache was populated, the positions column was not loaded". After this PR, the positions may also be absent because they were intentionally extracted during prewarm. Consider updating the comment to reflect both paths.

  2. _ => arm in prewarm_index_with_options (rust/lance/src/index.rs) — Since PrewarmOptions is #[non_exhaustive] with a single variant, the wildcard is required for compilation but currently unreachable. Fine for forward-compat, just noting it's dead code today.

No P0/P1 issues found. LGTM.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 91.80328% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-index/src/scalar/inverted/index.rs 92.45% 6 Missing and 2 partials ⚠️
rust/lance/src/index/api.rs 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

…g/fts-prewarmwithposition

# Conflicts:
#	python/src/dataset.rs
#	rust/lance-index/src/traits.rs
#	rust/lance/src/index.rs
@BubbleCal BubbleCal marked this pull request as ready for review April 9, 2026 06:51
@BubbleCal BubbleCal merged commit 5f4bf81 into main Apr 9, 2026
29 checks passed
@BubbleCal BubbleCal deleted the yang/fts-prewarmwithposition branch April 9, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-python Python bindings enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants