Skip to content

fix(fts): preserve parameter compatibility#7874

Merged
Xuanwo merged 1 commit into
mainfrom
xuanwo/fix-fts-param-compatibility
Jul 21, 2026
Merged

fix(fts): preserve parameter compatibility#7874
Xuanwo merged 1 commit into
mainfrom
xuanwo/fix-fts-param-compatibility

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

InvertedIndexParams is persisted in FTS index metadata and shared across Lance versions. PR #7681 added deny_unknown_fields, causing existing indexes that still contain retired fields such as skip_merge to fail to load. This also breaks bench_regress against its existing Wikipedia FTS index.

Restore Serde's tolerant-reader behavior so historical and future metadata fields are ignored while validation for recognized parameters remains intact. Regression coverage exercises both training JSON and the on-disk metadata.lance loading path.

@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer bug Something isn't working labels Jul 21, 2026
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 22399923-5ed7-44f5-9da2-96915d053d78

📥 Commits

Reviewing files that changed from the base of the PR and between 0deacf4 and 6f6cca1.

📒 Files selected for processing (2)
  • rust/lance-index/src/scalar/inverted/index.rs
  • rust/lance-index/src/scalar/inverted/tokenizer.rs

📝 Walkthrough

Walkthrough

Inverted-index parameter deserialization now ignores unknown JSON fields. Tests verify this behavior for training JSON normalization and metadata-based parameter loading.

Changes

Inverted-index parameter compatibility

Layer / File(s) Summary
Unknown-field tolerant parameter loading
rust/lance-index/src/scalar/inverted/tokenizer.rs, rust/lance-index/src/scalar/inverted/index.rs
Persisted training and metadata parameters ignore unrecognized JSON fields, while normalized output contains only supported parameters. Tests cover both loading paths.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • lance-format/lance#7816: Adds a related InvertedIndex::load_params parameter-reading path and tests for unknown JSON fields.

Suggested reviewers: bubblecal, luqqiu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: restoring FTS parameter compatibility.
Description check ✅ Passed The description accurately explains the compatibility fix and the added regression coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch xuanwo/fix-fts-param-compatibility

Comment @coderabbitai help to get the list of available commands.

@Xuanwo
Xuanwo marked this pull request as ready for review July 21, 2026 04:15
@Xuanwo
Xuanwo merged commit 81cd887 into main Jul 21, 2026
31 checks passed
@Xuanwo
Xuanwo deleted the xuanwo/fix-fts-param-compatibility branch July 21, 2026 04:18
wjones127 pushed a commit that referenced this pull request Jul 21, 2026
`InvertedIndexParams` is persisted in FTS index metadata and shared
across Lance versions. PR #7681 added `deny_unknown_fields`, causing
existing indexes that still contain retired fields such as `skip_merge`
to fail to load. This also breaks `bench_regress` against its existing
Wikipedia FTS index.

Restore Serde's tolerant-reader behavior so historical and future
metadata fields are ignored while validation for recognized parameters
remains intact. Regression coverage exercises both training JSON and the
on-disk `metadata.lance` loading path.

(cherry picked from commit 81cd887)
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants