Skip to content

feat: add mixed data file version capability - #8580

Merged
Xuanwo merged 3 commits into
mainfrom
xuanwo/mixed-file-versions-1-capability
Aug 27, 2026
Merged

feat: add mixed data file version capability#8580
Xuanwo merged 3 commits into
mainfrom
xuanwo/mixed-file-versions-1-capability

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Aug 17, 2026

Copy link
Copy Markdown
Member

Summary

  • reserve bit 256 for mixed V2 data file versions without adding it to the supported mask
  • keep this layer refusing mixed manifests until the per-file storage contract lands
  • add shared commit gates and sticky-capability plumbing needed for safe activation

Stack

  1. feat: add mixed data file version capability #8580 - capability reservation and commit gates
  2. feat: validate mixed V2 data file versions #8581 - capability activation, storage validation, and exact read dispatch
  3. feat: support per-operation V2 write targets #8582 - per-operation write targets
  4. feat: propagate exact V2 versions across dataset operations #8583 - dataset operation propagation
  5. feat: target exact V2 versions in compaction #8584 - compaction targets and binary copy policy
  6. feat: expose exact V2 versions in bindings #8585 - bindings, documentation, and rollout contract

Testing

  • cargo test -p lance-table feature_flags
  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings

@github-actions

Copy link
Copy Markdown
Contributor

Important

This PR touches the Lance format specification.

Substantive changes to the format specification — the .proto definitions
and the spec docs under docs/src/format/ — require a PMC vote before merge.
Minor edits such as typo fixes, wording, or formatting are excluded; use your
judgment.

If this is a meaningful format change:

  • Start a vote following the Lance community voting process.
    Format specification modifications need 3 binding +1 votes (excluding the
    proposer), held on GitHub Discussions, with a minimum voting period of 1 week.
  • Once the vote passes, link the completed vote in this PR. It should not be
    merged until the vote is linked.

@github-actions github-actions Bot added A-format On-disk format: protos and format spec docs A-namespace Namespace impls enhancement New feature or request labels Aug 17, 2026
@Xuanwo
Xuanwo force-pushed the xuanwo/mixed-file-versions-1-capability branch from 67d1f56 to 901e1e9 Compare August 18, 2026 18:37
@github-actions github-actions Bot added the A-python Python bindings label Aug 18, 2026
@Xuanwo
Xuanwo force-pushed the xuanwo/mixed-file-versions-1-capability branch 2 times, most recently from a6324df to a1ed170 Compare August 20, 2026 10:27
@Xuanwo
Xuanwo marked this pull request as ready for review August 20, 2026 13:43
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 20, 2026
@Xuanwo
Xuanwo force-pushed the xuanwo/mixed-file-versions-1-capability branch 3 times, most recently from bda4b4e to 187cc6d Compare August 20, 2026 14:26
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 20, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 20, 2026
@Xuanwo
Xuanwo force-pushed the xuanwo/mixed-file-versions-1-capability branch 2 times, most recently from 9ed0a00 to 9e8b45b Compare August 20, 2026 15:22
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 20, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 20, 2026
&Session::default(),
)
.await?;
ensure_can_write_manifest(&source_manifest)?;

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.

Could we move this compatibility check before clone-side effects? Dataset::deep_clone copies every data, deletion, and index file to the target before CommitBuilder reaches this line, and do_commit_new_dataset has already written the destination transaction file. A reader-compatible source with an unknown writer-only flag therefore returns NotSupported only after leaving a complete uncommitted clone in the target. Please preflight the selected source manifest before copying, while retaining this check for commit-time races.

@Xuanwo
Xuanwo force-pushed the xuanwo/mixed-file-versions-1-capability branch from 9e8b45b to 1cc5983 Compare August 25, 2026 07:50
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 25, 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: request changes.

The bit-256 reservation remains coherent, but this revision is not fail-closed at every boundary: refresh can install an unsupported manifest, namespace overwrite validates only after source flags have been discarded, and the existing clone preflight finding still rejects after copy side effects. The rebased Lance test target also no longer compiles.

A viable revision should validate the freshest source manifest before caching, derivation, staging, or clone copying; retain the final commit-time checks for races; and restore the missing test import.

use crate::session::Session;
use crate::session::caches::ManifestKey;
use crate::{Dataset, Error, Result};
use lance_table::format::{DataStorageFormat, Fragment};

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.

Removing Fragment breaks the rebased test target: the current base still uses it in install_fragments and test_get_fragment_on_legacy_manifest. Keep the import (or qualify those uses) so the new restore regression and the Lance lib test target compile.

Reproducer

cargo test -p lance test_restore_rejects_unknown_target_flags --lib

Expected: the focused regression runs.

Observed on 1cc598374e7989c7ba38de1b7b5a21c8a8e1b4a2: compilation fails with E0425 at dataset_io.rs:2962, :2999, and :3005 because Fragment is not in scope.

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.

Fixed in b8c333b: Fragment is back in scope, and the restore regression now compiles and passes.

review_feedback_thread_marker_template:

Comment thread rust/lance/src/dataset.rs Outdated
read_struct(object_reader.as_ref(), offset).await
}?;

validate_paired_feature_flags(&manifest)?;

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.

This validation covers load_manifest, but checkout_latest() calls latest_manifest(), which uses raw read_manifest, caches the result, and installs it without validate_paired_feature_flags or can_read_dataset. An already-open client can therefore refresh onto a paired bit-256 manifest and continue under legacy snapshot-wide dispatch. Route latest refresh/cache through the common validated load path, or validate before caching and installing.

Reproducer

I added a focused test that writes version 2 with auto_set_feature_flags: false, sets FLAG_UNKNOWN in both flag words, then calls dataset.checkout_latest() from version 1 and asserts Error::NotSupported.

cargo test -p lance test_checkout_latest_rejects_unknown_reader_flags --lib

Expected: Err(Error::NotSupported { .. }).

Observed on the current head: checkout_latest() returned Ok(()), so unwrap_err() panicked.

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.

The latest-refresh and cache paths are repaired, but reader admission is still incomplete: DatasetBuilder::with_serialized_manifest installs a decoded manifest without ensure_can_read_manifest. The focused serialized-manifest regression still loads paired unknown flags successfully, so this admission finding remains and has a current successor projection.

review_feedback_thread_marker_template:

indices: Option<Vec<IndexMetadata>>,
transaction: Transaction,
) -> std::result::Result<(), CommitError> {
ensure_can_write_manifest(manifest).map_err(CommitError::from)?;

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.

This gates the derived manifest after manifest_from_overwrite_transaction has called Manifest::new_from_previous, which preserves only sticky bit 256 and drops every other unknown writer flag. The earlier ensure_writable(dataset.metadata()) guards namespace table metadata, not Lance manifest flags. A reader-compatible manifest with an unknown writer-only bit can therefore be staged, cleared, then pass this check and republish its files as legacy-compatible. Gate dataset.manifest() before staging/derivation, while retaining this sink check for races.

Reproducer

I added a focused test that sets writer bit 1 << 9 on the source manifest, verifies ensure_can_write_manifest(&source) rejects it, derives the overwrite manifest with manifest_from_overwrite_transaction, and requires the same gate to reject the result.

cargo test -p lance-namespace-impls test_manifest_rewrite_preserves_unknown_writer_flags_until_gate --lib

Expected: the derived manifest remains unsupported.

Observed on the current head: the derived manifest had lost the flag, so ensure_can_write_manifest(&derived) returned Ok(()) and unwrap_err() panicked.

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.

The overwrite path now gates the source before staging, but declare_table still writes .lance-reserved before any ensure_manifest_writable call. In cargo test -p lance-namespace-impls test_declare_table_rejects_unknown_writer_flag_before_marker --lib, the operation returned NotSupported only after the marker existed, so the pre-side-effect namespace admission finding remains.

review_feedback_thread_marker_template:

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 25, 2026
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 25, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-changes Latest Gatekeeper recommendation requests changes. and removed K-changes Latest Gatekeeper recommendation requests changes. labels Aug 25, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-changes Latest Gatekeeper recommendation requests changes. and removed K-changes Latest Gatekeeper recommendation requests changes. labels Aug 26, 2026
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 26, 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 with a non-blocking risk.

The serialized-manifest admission blocker is fixed in 70b0275: preloaded manifests now cross the shared reader gate before installation.

The author accepts operation-wide orphaned staging artifacts as a deferred operational risk. No further change is requested for this pull request.

@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 26, 2026
@Xuanwo
Xuanwo merged commit 48eb6ff into main Aug 27, 2026
34 of 35 checks passed
@Xuanwo
Xuanwo deleted the xuanwo/mixed-file-versions-1-capability branch August 27, 2026 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-format On-disk format: protos and format spec docs A-namespace Namespace impls A-python Python bindings enhancement New feature or request K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants