From 790a33bfb29b89e0c6b7d4b3565cea494719774d Mon Sep 17 00:00:00 2001 From: Will Jones Date: Tue, 30 Jun 2026 12:30:57 -0700 Subject: [PATCH 1/3] feat(python): expose DataOverlay commit operation Adds LanceOperation.DataOverlay (with DataOverlayFile and DataOverlayGroup) so Python can commit data overlay files, mirroring the existing DataReplacement binding. A DataOverlayFile carries the value DataFile plus exactly one of shared_offsets (dense coverage) or field_offsets (sparse, per-field coverage); the commit stamps committed_version. Also fills in the overlays field on the Python FragmentMetadata -> Fragment conversion, which OSS-1322 left unset (overlays are committed via DataOverlay, not carried through fragment metadata). Co-Authored-By: Claude Opus 4.8 (1M context) --- python/python/lance/dataset.py | 59 +++++++++ python/python/tests/test_dataset.py | 181 +++++++++++++++++++++++++++- python/src/fragment.rs | 7 +- python/src/transaction.rs | 117 +++++++++++++++++- 4 files changed, 358 insertions(+), 6 deletions(-) diff --git a/python/python/lance/dataset.py b/python/python/lance/dataset.py index a1ce77b82b8..9e3c49a2e77 100644 --- a/python/python/lance/dataset.py +++ b/python/python/lance/dataset.py @@ -5991,6 +5991,65 @@ class DataReplacement(BaseOperation): replacements: List[LanceOperation.DataReplacementGroup] + @dataclass + class DataOverlayFile: + """ + An overlay file supplying new values for a subset of + ``(physical offset, field)`` cells of a fragment, resolved on read and + layered over the base data without rewriting the base files. + + The overlay is dense or sparse depending on the shape of ``offsets``: + pass a flat ``List[int]`` for a dense overlay (one offset set shared by + every field in ``data_file``) or a ``List[List[int]]`` for a sparse + overlay (one offset set per field, in the order of the file's fields). + Offsets are **physical** row offsets (positions in the base files, + counting deleted rows), like deletion vectors. + + Attributes + ---------- + data_file : DataFile + The Lance data file storing the overlay's new cell values — one + value column per covered field, with no row-offset key column. The + value at each covered offset is stored at the rank (0-based count of + covered offsets below it) of that offset in the field's coverage. + offsets : Union[List[int], List[List[int]]] + The covered physical row offsets. A flat list is dense coverage + (shared by every field); a list of per-field lists is sparse + coverage (in field order). + """ + + data_file: DataFile + offsets: Union[List[int], List[List[int]]] + + @dataclass + class DataOverlayGroup: + """ + Overlay files to append to a single fragment. + + Attributes + ---------- + fragment_id : int + The id of the fragment the overlays apply to. + overlays : List[LanceOperation.DataOverlayFile] + The overlay files to append, ordered oldest-first (a later entry is + newer and wins where coverage overlaps). + """ + + fragment_id: int + overlays: List[LanceOperation.DataOverlayFile] + + @dataclass + class DataOverlay(BaseOperation): + """ + Operation that appends data overlay files to fragments. + + Overlays are appended to each fragment's existing overlays (overlays + written by concurrent commits are preserved) and resolved on read, + newest-last, over the base data without rewriting it. + """ + + groups: List[LanceOperation.DataOverlayGroup] + @dataclass class Project(BaseOperation): """ diff --git a/python/python/tests/test_dataset.py b/python/python/tests/test_dataset.py index 500ee2e17bf..5856b2f8760 100644 --- a/python/python/tests/test_dataset.py +++ b/python/python/tests/test_dataset.py @@ -31,7 +31,7 @@ from lance.commit import CommitConflictError from lance.dataset import LANCE_COMMIT_MESSAGE_KEY, AutoCleanupConfig from lance.debug import format_fragment -from lance.file import stable_version +from lance.file import LanceFileWriter, stable_version from lance.schema import LanceSchema from lance.util import validate_vector_index @@ -4963,6 +4963,185 @@ def test_data_replacement(tmp_path: Path): assert tbl == expected +def _write_overlay_file( + dataset, base_dir: Path, name: str, batch: pa.Table, fields: List[int] +): + """Write an overlay value file (one value column per covered field, no key + column) and return a DataFile mapping its columns to the given dataset + `fields`. The file version is copied from a base data file.""" + path = base_dir / "data" / name + with LanceFileWriter(str(path)) as writer: + writer.write_batch(batch) + base_df = dataset.get_fragments()[0].metadata.files[0] + return lance.fragment.DataFile( + path=name, + fields=fields, + column_indices=list(range(len(fields))), + file_major_version=base_df.file_major_version, + file_minor_version=base_df.file_minor_version, + file_size_bytes=os.path.getsize(path), + ) + + +def test_data_overlay_dense(tmp_path: Path): + base_dir = tmp_path / "test" + table = pa.table( + { + "id": pa.array(range(10), pa.int32()), + "val": pa.array([i * 10 for i in range(10)], pa.int32()), + } + ) + dataset = lance.write_dataset(table, base_dir) + + # Overlay `val` at physical offsets {1, 4} with new values. + data_file = _write_overlay_file( + dataset, + base_dir, + "ov.lance", + pa.table({"val": pa.array([111, 444], pa.int32())}), + fields=[1], + ) + assert data_file.fields == [1] # `val` is field id 1 + + overlay = lance.LanceOperation.DataOverlayFile(data_file, offsets=[1, 4]) + op = lance.LanceOperation.DataOverlay( + [lance.LanceOperation.DataOverlayGroup(0, [overlay])] + ) + dataset = lance.LanceDataset.commit(dataset, op, read_version=dataset.version) + + result = dataset.to_table() + assert result.column("val").to_pylist() == [0, 111, 20, 30, 444, 50, 60, 70, 80, 90] + # The unrelated `id` column is untouched. + assert result.column("id").to_pylist() == list(range(10)) + + +def test_data_overlay_newest_wins(tmp_path: Path): + base_dir = tmp_path / "test" + table = pa.table( + { + "id": pa.array(range(10), pa.int32()), + "val": pa.array([i * 10 for i in range(10)], pa.int32()), + } + ) + dataset = lance.write_dataset(table, base_dir) + + older = _write_overlay_file( + dataset, + base_dir, + "older.lance", + pa.table({"val": pa.array([111, 444], pa.int32())}), + fields=[1], + ) + dataset = lance.LanceDataset.commit( + dataset, + lance.LanceOperation.DataOverlay( + [ + lance.LanceOperation.DataOverlayGroup( + 0, + [lance.LanceOperation.DataOverlayFile(older, offsets=[1, 4])], + ) + ] + ), + read_version=dataset.version, + ) + # A newer overlay re-covers offset 1; it must win there. + newer = _write_overlay_file( + dataset, + base_dir, + "newer.lance", + pa.table({"val": pa.array([999], pa.int32())}), + fields=[1], + ) + dataset = lance.LanceDataset.commit( + dataset, + lance.LanceOperation.DataOverlay( + [ + lance.LanceOperation.DataOverlayGroup( + 0, [lance.LanceOperation.DataOverlayFile(newer, offsets=[1])] + ) + ] + ), + read_version=dataset.version, + ) + + val = dataset.to_table().column("val").to_pylist() + assert val[1] == 999 # newest overlay wins + assert val[4] == 444 # only the older overlay covers offset 4 + + +def test_data_overlay_sparse_per_field(tmp_path: Path): + base_dir = tmp_path / "test" + table = pa.table( + { + "id": pa.array(range(10), pa.int32()), + "val": pa.array([i * 10 for i in range(10)], pa.int32()), + } + ) + dataset = lance.write_dataset(table, base_dir) + + # Sparse overlay: `id` covers offset {2}, `val` covers offset {3}. The value + # file carries one value per field (rank 0 of each field's coverage). + data_file = _write_overlay_file( + dataset, + base_dir, + "sparse.lance", + pa.table( + { + "id": pa.array([777], pa.int32()), + "val": pa.array([330], pa.int32()), + } + ), + fields=[0, 1], + ) + assert data_file.fields == [0, 1] + + overlay = lance.LanceOperation.DataOverlayFile(data_file, offsets=[[2], [3]]) + op = lance.LanceOperation.DataOverlay( + [lance.LanceOperation.DataOverlayGroup(0, [overlay])] + ) + dataset = lance.LanceDataset.commit(dataset, op, read_version=dataset.version) + + result = dataset.to_table() + assert result.column("id").to_pylist()[2] == 777 + assert result.column("val").to_pylist()[3] == 330 + # Fields resolve independently: id at offset 3 and val at offset 2 fall through. + assert result.column("id").to_pylist()[3] == 3 + assert result.column("val").to_pylist()[2] == 20 + + +def test_data_overlay_rejects_invalid_offsets(tmp_path: Path): + base_dir = tmp_path / "test" + table = pa.table({"val": pa.array([0, 1, 2], pa.int32())}) + dataset = lance.write_dataset(table, base_dir) + data_file = _write_overlay_file( + dataset, + base_dir, + "ov.lance", + pa.table({"val": pa.array([9], pa.int32())}), + fields=[0], + ) + + # offsets is neither a flat list of ints (dense) nor a list of per-field int + # lists (sparse), so the coverage shape can't be resolved. + with pytest.raises(ValueError, match="offsets must be a list"): + lance.LanceDataset.commit( + dataset, + lance.LanceOperation.DataOverlay( + [ + lance.LanceOperation.DataOverlayGroup( + 0, + [ + lance.LanceOperation.DataOverlayFile( + data_file, offsets=[0, [1]] + ) + ], + ) + ] + ), + read_version=dataset.version, + ) + + def test_schema_project_drop_column(tmp_path: Path): table = pa.Table.from_pydict({"a": range(100, 200), "b": range(300, 400)}) base_dir = tmp_path / "test" diff --git a/python/src/fragment.rs b/python/src/fragment.rs index 336c6a4cf51..21e28f010fd 100644 --- a/python/src/fragment.rs +++ b/python/src/fragment.rs @@ -825,9 +825,10 @@ impl FromPyObject<'_, '_> for PyLance { row_id_meta, last_updated_at_version_meta, created_at_version_meta, - // Overlays are not exposed to Python yet, and the reverse conversion - // does not export them, so this round-trip is overlay-free. - overlays: vec![], + // Python's FragmentMetadata does not carry overlays; they are added + // to a fragment via the DataOverlay commit operation, not through + // fragment metadata round-trips. + overlays: Vec::new(), })) } } diff --git a/python/src/transaction.rs b/python/src/transaction.rs index 1b659395099..96d62659741 100644 --- a/python/src/transaction.rs +++ b/python/src/transaction.rs @@ -7,10 +7,11 @@ use crate::utils::{PyLance, class_name, export_vec, extract_vec}; use arrow::pyarrow::PyArrowType; use arrow_schema::Schema as ArrowSchema; use lance::dataset::transaction::{ - DataReplacementGroup, Operation, RewriteGroup, RewrittenIndex, Transaction, UpdateMap, - UpdateMapEntry, UpdateMode, + DataOverlayGroup, DataReplacementGroup, Operation, RewriteGroup, RewrittenIndex, Transaction, + UpdateMap, UpdateMapEntry, UpdateMode, }; use lance::datatypes::Schema; +use lance_table::format::overlay::{DataOverlayFile, OverlayCoverage}; use lance_table::format::{BasePath, DataFile, Fragment, IndexFile, IndexMetadata}; use pyo3::exceptions::PyValueError; use pyo3::types::PySet; @@ -206,6 +207,104 @@ impl<'py> IntoPyObject<'py> for PyLance<&DataReplacementGroup> { } } +impl FromPyObject<'_, '_> for PyLance { + type Error = PyErr; + fn extract(ob: Borrowed<'_, '_, PyAny>) -> PyResult { + let data_file = ob.getattr("data_file")?.extract::>()?.0; + let offsets = ob.getattr("offsets")?; + + // A flat list of offsets is a dense overlay (one coverage shared by every + // field); a list of per-field lists is a sparse overlay. Differentiate by + // shape, trying the dense form first. + let coverage = if let Ok(shared) = offsets.extract::>() { + OverlayCoverage::dense(RoaringBitmap::from_iter(shared)) + } else if let Ok(per_field) = offsets.extract::>>() { + OverlayCoverage::sparse( + per_field + .into_iter() + .map(RoaringBitmap::from_iter) + .collect(), + ) + } else { + return Err(PyValueError::new_err( + "DataOverlayFile.offsets must be a list of ints (dense coverage shared by \ + every field) or a list of per-field int lists (sparse coverage)", + )); + }; + + Ok(Self(DataOverlayFile { + data_file, + coverage, + // The commit stamps the effective version; the value here is ignored. + committed_version: 0, + })) + } +} + +impl<'py> IntoPyObject<'py> for PyLance<&DataOverlayFile> { + type Target = PyAny; + type Output = Bound<'py, Self::Target>; + type Error = PyErr; + + fn into_pyobject(self, py: Python<'py>) -> Result { + let namespace = py + .import(intern!(py, "lance")) + .and_then(|module| module.getattr(intern!(py, "LanceOperation"))) + .expect("Failed to import LanceOperation namespace"); + + let data_file = PyLance(&self.0.data_file).into_pyobject(py)?; + let cls = namespace + .getattr("DataOverlayFile") + .expect("Failed to get DataOverlayFile class"); + + // Mirror the read side: a dense overlay becomes a flat list of offsets, a + // sparse overlay a list of per-field lists. + match &self.0.coverage { + OverlayCoverage::Shared(bitmap) => { + let offsets: Vec = bitmap.iter().collect(); + cls.call1((data_file, offsets)) + } + OverlayCoverage::PerField(bitmaps) => { + let offsets: Vec> = bitmaps.iter().map(|b| b.iter().collect()).collect(); + cls.call1((data_file, offsets)) + } + } + } +} + +impl FromPyObject<'_, '_> for PyLance { + type Error = PyErr; + fn extract(ob: Borrowed<'_, '_, PyAny>) -> PyResult { + let fragment_id = ob.getattr("fragment_id")?.extract::()?; + let overlays = extract_vec(&ob.getattr("overlays")?)?; + Ok(Self(DataOverlayGroup { + fragment_id, + overlays, + })) + } +} + +impl<'py> IntoPyObject<'py> for PyLance<&DataOverlayGroup> { + type Target = PyAny; + type Output = Bound<'py, Self::Target>; + type Error = PyErr; + + fn into_pyobject(self, py: Python<'py>) -> Result { + let namespace = py + .import(intern!(py, "lance")) + .and_then(|module| module.getattr(intern!(py, "LanceOperation"))) + .expect("Failed to import LanceOperation namespace"); + + let fragment_id = self.0.fragment_id; + let overlays = export_vec(py, self.0.overlays.as_slice())?; + + let cls = namespace + .getattr("DataOverlayGroup") + .expect("Failed to get DataOverlayGroup class"); + cls.call1((fragment_id, overlays)) + } +} + #[derive(Debug, Clone)] pub struct PyUpdateMode(pub UpdateMode); @@ -350,6 +449,13 @@ impl FromPyObject<'_, '_> for PyLance { Ok(Self(op)) } + "DataOverlay" => { + let groups = extract_vec(&ob.getattr("groups")?)?; + + let op = Operation::DataOverlay { groups }; + + Ok(Self(op)) + } "Project" => { let schema = extract_schema(&ob.getattr("schema")?)?; @@ -487,6 +593,13 @@ impl<'py> IntoPyObject<'py> for PyLance<&Operation> { .expect("Failed to get DataReplacement class"); cls.call1((replacements,)) } + Operation::DataOverlay { groups } => { + let groups = export_vec(py, groups.as_slice())?; + let cls = namespace + .getattr("DataOverlay") + .expect("Failed to get DataOverlay class"); + cls.call1((groups,)) + } Operation::Delete { updated_fragments, deleted_fragment_ids, From 968d024e6c6ce0ea0a84eb005e080770a8332ea0 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Mon, 13 Jul 2026 10:53:10 -0700 Subject: [PATCH 2/3] feat(python): round-trip fragment overlays; address review docs Addresses review feedback on #7540: - Reword the DataOverlayFile/DataOverlay docstrings: offset "list" not "set", drop the redundant "no row-offset key column" note, and clarify that the latest group wins when multiple groups target the same data. - Round-trip a fragment's overlays through FragmentMetadata so operations that pass existing fragments back (a manual Delete/Update/Merge commit) no longer silently drop them. FragmentMetadata gains an `overlays` field wired through to_json/from_json and both PyO3 conversions; DataOverlayFile gains `committed_version` (None on commit input, populated on read) so overlay precedence survives the round-trip. Adds test_data_overlay_round_trips_through_fragment_metadata covering the metadata, JSON, and commit round-trips. Co-Authored-By: Claude Opus 4.8 (1M context) --- python/python/lance/dataset.py | 23 +++++++++---- python/python/lance/fragment.py | 41 ++++++++++++++++++++-- python/python/tests/test_dataset.py | 53 +++++++++++++++++++++++++++++ python/src/fragment.rs | 11 +++--- python/src/transaction.rs | 17 ++++++--- 5 files changed, 127 insertions(+), 18 deletions(-) diff --git a/python/python/lance/dataset.py b/python/python/lance/dataset.py index 9e3c49a2e77..666c943dad3 100644 --- a/python/python/lance/dataset.py +++ b/python/python/lance/dataset.py @@ -5999,9 +5999,9 @@ class DataOverlayFile: layered over the base data without rewriting the base files. The overlay is dense or sparse depending on the shape of ``offsets``: - pass a flat ``List[int]`` for a dense overlay (one offset set shared by + pass a flat ``List[int]`` for a dense overlay (one offset list shared by every field in ``data_file``) or a ``List[List[int]]`` for a sparse - overlay (one offset set per field, in the order of the file's fields). + overlay (one offset list per field, in the order of the file's fields). Offsets are **physical** row offsets (positions in the base files, counting deleted rows), like deletion vectors. @@ -6009,17 +6009,23 @@ class DataOverlayFile: ---------- data_file : DataFile The Lance data file storing the overlay's new cell values — one - value column per covered field, with no row-offset key column. The - value at each covered offset is stored at the rank (0-based count of - covered offsets below it) of that offset in the field's coverage. + value column per covered field. The value at each covered offset is + stored at the rank (0-based count of covered offsets below it) of + that offset in the field's coverage. offsets : Union[List[int], List[List[int]]] The covered physical row offsets. A flat list is dense coverage (shared by every field); a list of per-field lists is sparse coverage (in field order). + committed_version : Optional[int] + The dataset version at which this overlay became effective. Leave as + ``None`` when creating an overlay to commit — the commit stamps it. + It is populated when reading an existing fragment's overlays so they + round-trip through :class:`FragmentMetadata`. """ data_file: DataFile offsets: Union[List[int], List[List[int]]] + committed_version: Optional[int] = None @dataclass class DataOverlayGroup: @@ -6044,8 +6050,11 @@ class DataOverlay(BaseOperation): Operation that appends data overlay files to fragments. Overlays are appended to each fragment's existing overlays (overlays - written by concurrent commits are preserved) and resolved on read, - newest-last, over the base data without rewriting it. + written by concurrent commits are preserved) and resolved on read + over the base data without rewriting it. + + If multiple groups target the same data then the values in the + latest group take precedence. """ groups: List[LanceOperation.DataOverlayGroup] diff --git a/python/python/lance/fragment.py b/python/python/lance/fragment.py index adf220e59f6..bca62e2b280 100644 --- a/python/python/lance/fragment.py +++ b/python/python/lance/fragment.py @@ -45,6 +45,7 @@ ColumnOrdering, DatasetBasePath, LanceDataset, + LanceOperation, LanceScanner, ReaderLike, Transaction, @@ -78,6 +79,11 @@ class FragmentMetadata: The row created at version metadata, if any. last_updated_at_version_meta : Optional[RowDatasetVersionMeta] The row last updated at version metadata, if any. + overlays : List[LanceOperation.DataOverlayFile] + The data overlay files layered over this fragment's base data, if any. + Overlays are created via :class:`LanceOperation.DataOverlay`; they are + carried here so they survive operations that round-trip fragment + metadata (e.g. a manual ``Delete``, ``Update``, or ``Merge`` commit). """ id: int @@ -87,6 +93,7 @@ class FragmentMetadata: row_id_meta: Optional[RowIdMeta] = None created_at_version_meta: Optional[RowDatasetVersionMeta] = None last_updated_at_version_meta: Optional[RowDatasetVersionMeta] = None + overlays: List["LanceOperation.DataOverlayFile"] = field(default_factory=list) @property def num_deletions(self) -> int: @@ -110,12 +117,25 @@ def data_files(self) -> List[DataFile]: def to_json(self) -> dict: """Get this as a simple JSON-serializable dictionary.""" - files = [asdict(f) for f in self.files] - for f in files: - f["path"] = f.pop("_path") + + def _data_file_to_json(f: DataFile) -> dict: + d = asdict(f) + d["path"] = d.pop("_path") + return d + + files = [_data_file_to_json(f) for f in self.files] + overlays = [ + dict( + data_file=_data_file_to_json(o.data_file), + offsets=o.offsets, + committed_version=o.committed_version, + ) + for o in self.overlays + ] return dict( id=self.id, files=files, + overlays=overlays, physical_rows=self.physical_rows, deletion_file=( self.deletion_file.asdict() if self.deletion_file is not None else None @@ -159,6 +179,20 @@ def from_json(json_data: str) -> FragmentMetadata: json.dumps(last_updated_at_version_meta) ) + overlays = [] + overlays_json = json_data.get("overlays") + if overlays_json: + from .dataset import LanceOperation + + overlays = [ + LanceOperation.DataOverlayFile( + data_file=DataFile(**o["data_file"]), + offsets=o["offsets"], + committed_version=o.get("committed_version"), + ) + for o in overlays_json + ] + return FragmentMetadata( id=json_data["id"], files=[DataFile(**f) for f in json_data["files"]], @@ -167,6 +201,7 @@ def from_json(json_data: str) -> FragmentMetadata: row_id_meta=row_id_meta, created_at_version_meta=created_at_version_meta, last_updated_at_version_meta=last_updated_at_version_meta, + overlays=overlays, ) diff --git a/python/python/tests/test_dataset.py b/python/python/tests/test_dataset.py index 5856b2f8760..ba0c1758c03 100644 --- a/python/python/tests/test_dataset.py +++ b/python/python/tests/test_dataset.py @@ -5109,6 +5109,59 @@ def test_data_overlay_sparse_per_field(tmp_path: Path): assert result.column("val").to_pylist()[2] == 20 +def test_data_overlay_round_trips_through_fragment_metadata(tmp_path: Path): + import json + + base_dir = tmp_path / "test" + table = pa.table( + { + "id": pa.array(range(10), pa.int32()), + "val": pa.array([i * 10 for i in range(10)], pa.int32()), + } + ) + dataset = lance.write_dataset(table, base_dir) + + data_file = _write_overlay_file( + dataset, + base_dir, + "ov.lance", + pa.table({"val": pa.array([111, 444], pa.int32())}), + fields=[1], + ) + overlay = lance.LanceOperation.DataOverlayFile(data_file, offsets=[1, 4]) + dataset = lance.LanceDataset.commit( + dataset, + lance.LanceOperation.DataOverlay( + [lance.LanceOperation.DataOverlayGroup(0, [overlay])] + ), + read_version=dataset.version, + ) + overlay_version = dataset.version + + # Reading the fragment surfaces its overlays, stamped with the commit version. + metadata = dataset.get_fragments()[0].metadata + assert len(metadata.overlays) == 1 + assert metadata.overlays[0].offsets == [1, 4] + assert metadata.overlays[0].committed_version == overlay_version + + # The overlays survive a JSON round-trip of the metadata. + restored = lance.fragment.FragmentMetadata.from_json(json.dumps(metadata.to_json())) + assert len(restored.overlays) == 1 + assert restored.overlays[0].offsets == [1, 4] + assert restored.overlays[0].committed_version == overlay_version + + # A commit that round-trips the fragment (here an Overwrite) must keep the + # overlays, so the overlay still resolves on read instead of being dropped. + dataset = lance.LanceDataset.commit( + dataset, + lance.LanceOperation.Overwrite(dataset.schema, [restored]), + read_version=dataset.version, + ) + result = dataset.to_table() + assert result.column("val").to_pylist() == [0, 111, 20, 30, 444, 50, 60, 70, 80, 90] + assert result.column("id").to_pylist() == list(range(10)) + + def test_data_overlay_rejects_invalid_offsets(tmp_path: Path): base_dir = tmp_path / "test" table = pa.table({"val": pa.array([0, 1, 2], pa.int32())}) diff --git a/python/src/fragment.rs b/python/src/fragment.rs index 21e28f010fd..6b832870280 100644 --- a/python/src/fragment.rs +++ b/python/src/fragment.rs @@ -26,6 +26,7 @@ use lance::dataset::transaction::{Operation, Transaction}; use lance::dataset::{InsertBuilder, NewColumnTransform, WriteParams}; use lance_core::datatypes::BlobHandling; use lance_io::utils::CachedFileSize; +use lance_table::format::overlay::DataOverlayFile; use lance_table::format::{ DataFile, DeletionFile, DeletionFileType, Fragment, RowDatasetVersionMeta, RowIdMeta, }; @@ -825,10 +826,10 @@ impl FromPyObject<'_, '_> for PyLance { row_id_meta, last_updated_at_version_meta, created_at_version_meta, - // Python's FragmentMetadata does not carry overlays; they are added - // to a fragment via the DataOverlay commit operation, not through - // fragment metadata round-trips. - overlays: Vec::new(), + // Round-tripped so overlays survive operations that pass existing + // fragments back (a manual Delete/Update/Merge commit). Sorting + // newest-last is deferred to the manifest reload after commit. + overlays: extract_vec::(&ob.getattr("overlays")?)?, })) } } @@ -861,6 +862,7 @@ impl<'py> IntoPyObject<'py> for PyLance<&Fragment> { .created_at_version_meta .as_ref() .map(|r| PyRowDatasetVersionMeta(r.clone())); + let overlays = export_vec(py, &self.0.overlays)?; cls.call1(( self.0.id, @@ -870,6 +872,7 @@ impl<'py> IntoPyObject<'py> for PyLance<&Fragment> { row_id_meta, created_at_version_meta, last_updated_at_version_meta, + overlays, )) } } diff --git a/python/src/transaction.rs b/python/src/transaction.rs index 96d62659741..8443cee9712 100644 --- a/python/src/transaction.rs +++ b/python/src/transaction.rs @@ -232,11 +232,18 @@ impl FromPyObject<'_, '_> for PyLance { )); }; + // Present (and preserved) when round-tripping an existing fragment's + // overlays; None/0 when creating an overlay to commit, since the + // DataOverlay commit stamps the effective version. + let committed_version = ob + .getattr("committed_version")? + .extract::>()? + .unwrap_or(0); + Ok(Self(DataOverlayFile { data_file, coverage, - // The commit stamps the effective version; the value here is ignored. - committed_version: 0, + committed_version, })) } } @@ -257,16 +264,18 @@ impl<'py> IntoPyObject<'py> for PyLance<&DataOverlayFile> { .getattr("DataOverlayFile") .expect("Failed to get DataOverlayFile class"); + let committed_version = self.0.committed_version; + // Mirror the read side: a dense overlay becomes a flat list of offsets, a // sparse overlay a list of per-field lists. match &self.0.coverage { OverlayCoverage::Shared(bitmap) => { let offsets: Vec = bitmap.iter().collect(); - cls.call1((data_file, offsets)) + cls.call1((data_file, offsets, committed_version)) } OverlayCoverage::PerField(bitmaps) => { let offsets: Vec> = bitmaps.iter().map(|b| b.iter().collect()).collect(); - cls.call1((data_file, offsets)) + cls.call1((data_file, offsets, committed_version)) } } } From de8c63c8ee5270f3b964f5775cbea3e1bf51c62a Mon Sep 17 00:00:00 2001 From: Will Jones Date: Wed, 15 Jul 2026 15:15:38 -0700 Subject: [PATCH 3/3] fix(python): reject unsorted overlay offsets; fix fragment repr test The overlay `offsets` list maps positionally to value rows in `data_file`, but `RoaringBitmap::from_iter` silently sorts and dedups the offsets, so a non-ascending or duplicated list would corrupt that mapping without error. Validate that each dense/sparse offset list is strictly ascending and raise a clear ValueError otherwise. A proper RoaringBitmap Python binding (#7695) will supersede this. Also update `test_fragment_meta` to expect the new `overlays=[]` field in the `FragmentMetadata` repr, which was failing Python CI. Co-Authored-By: Claude Opus 4.8 (1M context) --- python/python/lance/dataset.py | 4 ++- python/python/tests/test_dataset.py | 43 ++++++++++++++++++++++++++++ python/python/tests/test_fragment.py | 2 +- python/src/transaction.rs | 21 ++++++++++++-- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/python/python/lance/dataset.py b/python/python/lance/dataset.py index 666c943dad3..def635d58a6 100644 --- a/python/python/lance/dataset.py +++ b/python/python/lance/dataset.py @@ -6015,7 +6015,9 @@ class DataOverlayFile: offsets : Union[List[int], List[List[int]]] The covered physical row offsets. A flat list is dense coverage (shared by every field); a list of per-field lists is sparse - coverage (in field order). + coverage (in field order). Each list must be strictly ascending + with no duplicates, since the Nth offset maps to the Nth value row + in ``data_file``; a non-ascending list raises ``ValueError``. committed_version : Optional[int] The dataset version at which this overlay became effective. Leave as ``None`` when creating an overlay to commit — the commit stamps it. diff --git a/python/python/tests/test_dataset.py b/python/python/tests/test_dataset.py index ba0c1758c03..898bdf93069 100644 --- a/python/python/tests/test_dataset.py +++ b/python/python/tests/test_dataset.py @@ -5195,6 +5195,49 @@ def test_data_overlay_rejects_invalid_offsets(tmp_path: Path): ) +@pytest.mark.parametrize( + "offsets", + [ + [2, 1], # dense, descending + [1, 1], # dense, duplicate + [[2, 1]], # sparse, descending + [[1, 1]], # sparse, duplicate + ], +) +def test_data_overlay_rejects_unsorted_offsets(tmp_path: Path, offsets): + # Offsets map positionally to value rows in data_file. A RoaringBitmap would + # silently reorder/dedup them, so a non-ascending list must be rejected up + # front rather than corrupting the row mapping. + base_dir = tmp_path / "test" + table = pa.table({"val": pa.array([0, 1, 2], pa.int32())}) + dataset = lance.write_dataset(table, base_dir) + data_file = _write_overlay_file( + dataset, + base_dir, + "ov.lance", + pa.table({"val": pa.array([9, 9], pa.int32())}), + fields=[0], + ) + + with pytest.raises(ValueError, match="strictly ascending"): + lance.LanceDataset.commit( + dataset, + lance.LanceOperation.DataOverlay( + [ + lance.LanceOperation.DataOverlayGroup( + 0, + [ + lance.LanceOperation.DataOverlayFile( + data_file, offsets=offsets + ) + ], + ) + ] + ), + read_version=dataset.version, + ) + + def test_schema_project_drop_column(tmp_path: Path): table = pa.Table.from_pydict({"a": range(100, 200), "b": range(300, 400)}) base_dir = tmp_path / "test" diff --git a/python/python/tests/test_fragment.py b/python/python/tests/test_fragment.py index e0030eee654..11276a2213d 100644 --- a/python/python/tests/test_fragment.py +++ b/python/python/tests/test_fragment.py @@ -279,7 +279,7 @@ def test_fragment_meta(): "file_size_bytes=100), DataFile(path='1.lance', fields=[1], column_indices=[], " "file_major_version=0, file_minor_version=0, file_size_bytes=None)], " "physical_rows=100, deletion_file=None, row_id_meta=None, " - "created_at_version_meta=None, last_updated_at_version_meta=None)" + "created_at_version_meta=None, last_updated_at_version_meta=None, overlays=[])" ) diff --git a/python/src/transaction.rs b/python/src/transaction.rs index 8443cee9712..37085d60de7 100644 --- a/python/src/transaction.rs +++ b/python/src/transaction.rs @@ -207,6 +207,21 @@ impl<'py> IntoPyObject<'py> for PyLance<&DataReplacementGroup> { } } +// The Nth offset in an overlay list positionally maps to the Nth value row in +// `data_file`, but `RoaringBitmap` stores offsets in ascending order and drops +// duplicates. A caller-supplied list that isn't strictly ascending would be +// silently reordered, breaking that mapping, so reject it here instead. This can +// go away once we expose RoaringBitmap directly to Python (issue #7695). +fn bitmap_from_sorted_offsets(offsets: Vec) -> PyResult { + if offsets.windows(2).any(|w| w[0] >= w[1]) { + return Err(PyValueError::new_err( + "DataOverlayFile.offsets must be strictly ascending with no duplicates; \ + each offset positionally maps to a value row in data_file", + )); + } + Ok(RoaringBitmap::from_sorted_iter(offsets).expect("offsets verified strictly ascending")) +} + impl FromPyObject<'_, '_> for PyLance { type Error = PyErr; fn extract(ob: Borrowed<'_, '_, PyAny>) -> PyResult { @@ -217,13 +232,13 @@ impl FromPyObject<'_, '_> for PyLance { // field); a list of per-field lists is a sparse overlay. Differentiate by // shape, trying the dense form first. let coverage = if let Ok(shared) = offsets.extract::>() { - OverlayCoverage::dense(RoaringBitmap::from_iter(shared)) + OverlayCoverage::dense(bitmap_from_sorted_offsets(shared)?) } else if let Ok(per_field) = offsets.extract::>>() { OverlayCoverage::sparse( per_field .into_iter() - .map(RoaringBitmap::from_iter) - .collect(), + .map(bitmap_from_sorted_offsets) + .collect::>>()?, ) } else { return Err(PyValueError::new_err(