Skip to content

fix(dataset): track fragment add-columns cleanup - #8402

Open
lance-gatefixer[bot] wants to merge 3 commits into
mainfrom
gatekeeper/fix-7231-1
Open

fix(dataset): track fragment add-columns cleanup#8402
lance-gatefixer[bot] wants to merge 3 commits into
mainfrom
gatekeeper/fix-7231-1

Conversation

@lance-gatefixer

Copy link
Copy Markdown
Contributor

Summary

  • add a cleanup-aware FileFragment add-columns API that preserves ownership of staged files without changing the existing API
  • expose the cleanup token through LanceFragment.merge_columns with return_cleanup=True
  • cover stale outer-merge failure cleanup, original-file preservation, unrelated committed files, and external blob sources

Root cause

FileFragment::add_columns discarded the fragments_to_cleanup metadata returned by schema evolution. Fragment-level callers therefore had no safe way to distinguish files staged by their operation from pre-existing or independently committed files after a later Merge failed.

Fix

The new cleanup-aware API returns the staged fragment, schema, and an explicit cleanup token. Cleanup is opt-in and never runs on drop because a failed commit response can be ambiguous. The existing Rust and Python APIs retain their prior return values unless callers explicitly request cleanup ownership.

Validation

  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings
  • cargo test -p lance test_fragment_add_columns_cleanup_owns_only_staged_files
  • cargo test -p lance --doc add_columns_with_cleanup
  • uv run make build
  • uv run make lint
  • uv run pytest python/tests/test_schema_evolution.py (22 passed)

Fixes #7231

@github-actions github-actions Bot added A-python Python bindings bug Something isn't working labels Aug 7, 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.

The change closes the fragment-level cleanup ownership gap at the correct boundary: callers can retain a single-use explicit token through the outer merge, while existing Rust and Python return shapes stay compatible and ambiguous commit outcomes never trigger automatic deletion. Cleanup remains scoped to the staged (base_id, path) set, and the stale-merge regression verifies preservation of original, concurrent, and external data.

# Conflicts:
#	rust/lance/src/dataset/fragment.rs

@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.

The merge from main preserves the explicit staged-file cleanup ownership and correctly integrates the new nullability-conflict metadata. Fragment-level outer merges remain conservatively safe when that optimization signal is unavailable, while the existing Rust and Python return shapes and cleanup behavior remain unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-python Python bindings bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Track cleanup ownership for Fragment::add_columns after outer commit failures

0 participants