test: cover FTS v1 and v2 compatibility#7890
Conversation
📝 WalkthroughWalkthroughAdds v3.0.1 FTS v1 and v4.0.1 FTS v2 test fixtures, generation scripts, documentation, Git tracking rules, and parameterized Rust tests covering index versions, keyword queries, and phrase queries. ChangesFTS fixture compatibility
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FixtureGenerator
participant LanceDataset
participant FTSIndex
participant RustFixtureTest
FixtureGenerator->>LanceDataset: Write versioned dataset
LanceDataset->>FTSIndex: Create inverted text index
RustFixtureTest->>LanceDataset: Open fixture dataset
RustFixtureTest->>FTSIndex: Execute keyword and phrase queries
FTSIndex-->>RustFixtureTest: Return index version and matched row IDs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
test_data/v3.0.1/datagen.py-16-16 (1)
16-16: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReplace the version assert with an explicit check
This guard can be stripped in optimized runs, so the generator could rewrite the frozen fixture with the wrong Lance version. Raise a
RuntimeErrorwith the expected and actual versions instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test_data/v3.0.1/datagen.py` at line 16, Replace the assert comparing lance.__version__ with EXPECTED_LANCE_VERSION in the generator setup with an explicit conditional check. Raise RuntimeError when the versions differ, including both expected and actual version values in the message, so validation remains active in optimized runs.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Other comments:
In `@test_data/v3.0.1/datagen.py`:
- Line 16: Replace the assert comparing lance.__version__ with
EXPECTED_LANCE_VERSION in the generator setup with an explicit conditional
check. Raise RuntimeError when the versions differ, including both expected and
actual version values in the message, so validation remains active in optimized
runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: e9d8b1de-7330-4d68-9a6d-08f646cad363
📒 Files selected for processing (23)
.gitignorerust/lance/src/index.rstest_data/readme.mdtest_data/v3.0.1/datagen.pytest_data/v3.0.1/fts_v1/_indices/450d1cd4-9eb2-4a11-8eac-ab1b64b1ea31/metadata.lancetest_data/v3.0.1/fts_v1/_indices/450d1cd4-9eb2-4a11-8eac-ab1b64b1ea31/part_1_docs.lancetest_data/v3.0.1/fts_v1/_indices/450d1cd4-9eb2-4a11-8eac-ab1b64b1ea31/part_1_invert.lancetest_data/v3.0.1/fts_v1/_indices/450d1cd4-9eb2-4a11-8eac-ab1b64b1ea31/part_1_tokens.lancetest_data/v3.0.1/fts_v1/_transactions/0-0e238eef-4390-4c40-9c7e-3c1437b4735b.txntest_data/v3.0.1/fts_v1/_transactions/1-d331315a-f1a6-4735-a453-90470239bd60.txntest_data/v3.0.1/fts_v1/_versions/18446744073709551613.manifesttest_data/v3.0.1/fts_v1/_versions/18446744073709551614.manifesttest_data/v3.0.1/fts_v1/data/110110010101010101010101b80a6f4c4d804a62b40de56ffb.lancetest_data/v4.0.1/datagen.pytest_data/v4.0.1/fts_v2/_indices/1fb8e405-43d0-4707-be37-64eadc3d5e97/metadata.lancetest_data/v4.0.1/fts_v2/_indices/1fb8e405-43d0-4707-be37-64eadc3d5e97/part_6_docs.lancetest_data/v4.0.1/fts_v2/_indices/1fb8e405-43d0-4707-be37-64eadc3d5e97/part_6_invert.lancetest_data/v4.0.1/fts_v2/_indices/1fb8e405-43d0-4707-be37-64eadc3d5e97/part_6_tokens.lancetest_data/v4.0.1/fts_v2/_transactions/0-fbd896dd-b250-46fa-b60d-9b1474d7fc7b.txntest_data/v4.0.1/fts_v2/_transactions/1-ebe80000-4421-4ed6-8072-093abeb55eaf.txntest_data/v4.0.1/fts_v2/_versions/18446744073709551613.manifesttest_data/v4.0.1/fts_v2/_versions/18446744073709551614.manifesttest_data/v4.0.1/fts_v2/data/000100000010011011000110c47acb4569aa4d2c1957105328.lance
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
FTS compatibility did not have frozen on-disk fixtures, so changes to persisted index metadata or layouts could regress reading old indices without an end-to-end test. This is particularly important for FTS v1, whose metadata contains the retired
skip_mergeparameter.This PR adds fixtures written by stable Lance 3.0.1 and 4.0.1 for FTS layouts v1 and v2. The current reader verifies the stored index version, then executes a posting-list query that crosses a block boundary and a phrase query that reads positions. FTS v3 is intentionally deferred until it is written by a stable release.
The repository now explicitly permits
.lancefiles undertest_dataso each fixture includes real manifests, data files, and index payloads alongside a version-pinned generator.