refactor: activate exact current-format writers - #8023
Conversation
…mat-identity # Conflicts: # rust/lance-file/src/reader.rs # rust/lance/src/dataset/transaction.rs
… xuanwo/exact-version-stack-05-file-runtime
…rsion-stack-02-fixtures
…ct-version-stack-03-v1
…sion-stack-04-encoding-mechanisms
… xuanwo/exact-version-stack-05-file-runtime
…/exact-version-stack-06-writers
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
The exact-version dispatch fixes a real format-safety boundary: encoding selection and footer identity now have one owner, and the Rust, dataset, index, Python, and Java paths resolve selectors before construction. Stable V2.0–V2.2 fixtures retained byte identity, V2.3 remained deterministic, and the migrated binary-copy and shuffle paths passed targeted tests.
The remaining risk is the intentional removal of low-level Rust writer APIs without a deprecation bridge, which conflicts with this repository’s public-API policy and shifts migration cost to downstream users. If maintainers accept that compatibility tradeoff, I found no format or correctness blocker.
BubbleCal
left a comment
There was a problem hiding this comment.
The exact-version writer dispatch looks good overall. I left one question about the removed regression coverage.
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
Was removing this entire regression-test module intentional? I could not find equivalent coverage under the new version-specific writers for unequal-length column writes, page-size limits, compression overrides, or page-metadata spilling. Could we migrate these tests instead of dropping them?
There was a problem hiding this comment.
Yes, this was an artifact of the stacked PR boundary. The tests are migrated in the reader PR, but you are right that this PR should not temporarily drop the writer regression coverage. I’ll move the existing writer tests into a standalone test module here, keep them on the current reader API, and leave only the reader-side migration to the next PR.
Part 7/12 of #7877. Depends on #8023. This is an independently reviewable step toward the final layout demonstrated in #7979. This PR gives V2.0, V2.1, V2.2, and V2.3 explicit reader composition alongside V1. The root reader keeps shared runtime machinery and type-erased forwarding, but exact version modules own construction, decoder selection, and supported capabilities. The large shared reader implementation is reduced to mechanisms that are actually common. Version-sensitive choices no longer depend on ordering comparisons or scattered conditions, and the reader still parses persisted identity once at the boundary. Validation: * `cargo test -p lance-file` * `cargo clippy --all --tests --benches -- -D warnings` * `uv run make build` in `python/` * `cargo check --manifest-path java/lance-jni/Cargo.toml`
Part 6/12 of #7877. Depends on #8022.
This is an independently reviewable step toward the final layout demonstrated in #7979.
This PR switches the public file writer boundary to exact-version dispatch. The root
FileWriterbecomes type erasure plus forwarding, while V1 and each current format own construction and behavior inside their version modules. Writer call sites in Lance, indexes, Python, and Java now pass an exactConcreteFileVersion.This removes version-conditioned behavior from the shared current-format writer and makes every temporary, shuffle, merge, and dataset file creation site choose a format explicitly. Reader dispatch is intentionally deferred to the next PR.
Validation:
cargo clippy --all --tests --benches -- -D warningsuv run make buildinpython/cargo check --manifest-path java/lance-jni/Cargo.toml