Skip to content

Commit c528cf6

Browse files
authored
Support range-based reads for deletion vectors (#3478)
* Support range-based reads for deletion vectors * Test external single and packed deletion vector fixtures * Make delete file sets read-only and clarify DV validation * Keep DV range identity internal to scan aggregation Restore native scan task sets by routing deletion vectors to their referenced data files. Preserve REST range references and cover local and REST scans of packed DV fixtures. * Preserve validated DV field types and keep helper internal * Retrigger CI
1 parent eb30d8a commit c528cf6

10 files changed

Lines changed: 699 additions & 26 deletions

File tree

‎pyiceberg/io/pyarrow.py‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,10 @@
149149
visit_with_partner,
150150
)
151151
from pyiceberg.table import DOWNCAST_NS_TIMESTAMP_TO_US_ON_WRITE, TableProperties
152-
from pyiceberg.table.deletion_vector import deletion_vectors_from_puffin_file
152+
from pyiceberg.table.deletion_vector import read_deletion_vectors
153153
from pyiceberg.table.locations import load_location_provider
154154
from pyiceberg.table.metadata import TableMetadata
155155
from pyiceberg.table.name_mapping import NameMapping, apply_name_mapping
156-
from pyiceberg.table.puffin import PuffinFile
157156
from pyiceberg.transforms import IdentityTransform, TruncateTransform
158157
from pyiceberg.typedef import EMPTY_DICT, Properties, Record, TableVersion
159158
from pyiceberg.types import (
@@ -1175,10 +1174,7 @@ def _read_deletes(io: FileIO, data_file: DataFile) -> dict[str, pa.ChunkedArray]
11751174
for path in table.column("file_path").unique()
11761175
}
11771176
elif data_file.file_format == FileFormat.PUFFIN:
1178-
with io.new_input(data_file.file_path).open() as fi:
1179-
payload = fi.read()
1180-
1181-
return {dv.referenced_data_file: dv.to_vector() for dv in deletion_vectors_from_puffin_file(PuffinFile(payload))}
1177+
return {dv.referenced_data_file: dv.to_vector() for dv in read_deletion_vectors(io, data_file)}
11821178
else:
11831179
raise ValueError(f"Delete file format not supported: {data_file.file_format}")
11841180

@@ -1745,14 +1741,28 @@ def _task_to_record_batches(
17451741
)
17461742

17471743

1744+
@dataclass(frozen=True, slots=True)
1745+
class _DeleteFileKey:
1746+
"""Identity of a delete file or a referenced DV range."""
1747+
1748+
file_path: str
1749+
content_offset: int | None
1750+
content_size_in_bytes: int | None
1751+
1752+
17481753
def _read_all_delete_files(io: FileIO, tasks: Iterable[FileScanTask]) -> dict[str, list[ChunkedArray]]:
17491754
deletes_per_file: dict[str, list[ChunkedArray]] = {}
1750-
unique_deletes = set(itertools.chain.from_iterable([task.delete_files for task in tasks]))
1755+
# DataFile equality uses only the path; DVs from different tasks may share a file.
1756+
unique_deletes: dict[_DeleteFileKey, DataFile] = {}
1757+
for task in tasks:
1758+
for delete_file in task.delete_files:
1759+
key = _DeleteFileKey(delete_file.file_path, delete_file.content_offset, delete_file.content_size_in_bytes)
1760+
unique_deletes.setdefault(key, delete_file)
17511761
if len(unique_deletes) > 0:
17521762
executor = ExecutorFactory.get_or_create()
17531763
deletes_per_files: Iterator[dict[str, ChunkedArray]] = executor.map(
17541764
lambda args: _read_deletes(*args),
1755-
[(io, delete_file) for delete_file in unique_deletes],
1765+
[(io, delete_file) for delete_file in unique_deletes.values()],
17561766
)
17571767
for delete in deletes_per_files:
17581768
for file, arr in delete.items():

‎pyiceberg/table/__init__.py‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,15 @@
4444
manifest_evaluator,
4545
)
4646
from pyiceberg.io import FileIO, load_file_io
47-
from pyiceberg.manifest import DataFile, DataFileContent, ManifestContent, ManifestEntry, ManifestEntryStatus, ManifestFile
47+
from pyiceberg.manifest import (
48+
DataFile,
49+
DataFileContent,
50+
FileFormat,
51+
ManifestContent,
52+
ManifestEntry,
53+
ManifestEntryStatus,
54+
ManifestFile,
55+
)
4856
from pyiceberg.partitioning import PARTITION_FIELD_ID_START, UNPARTITIONED_PARTITION_SPEC, PartitionKey, PartitionSpec
4957
from pyiceberg.schema import Schema
5058
from pyiceberg.table.delete_file_index import DeleteFileIndex
@@ -2277,7 +2285,7 @@ def from_rest_response(
22772285
delete_file = delete_files[idx]
22782286
if isinstance(delete_file, RESTEqualityDeleteFile):
22792287
raise NotImplementedError(f"PyIceberg does not yet support equality deletes: {delete_file.file_path}")
2280-
resolved_deletes.add(_rest_file_to_data_file(delete_file))
2288+
resolved_deletes.add(_rest_file_to_data_file(delete_file, default_referenced_data_file=data_file.file_path))
22812289

22822290
return FileScanTask(
22832291
data_file=data_file,
@@ -2286,9 +2294,9 @@ def from_rest_response(
22862294
)
22872295

22882296

2289-
def _rest_file_to_data_file(rest_file: RESTContentFile) -> DataFile:
2297+
def _rest_file_to_data_file(rest_file: RESTContentFile, default_referenced_data_file: str | None = None) -> DataFile:
22902298
"""Convert a REST content file to a manifest DataFile."""
2291-
from pyiceberg.catalog.rest.scan_planning import RESTDataFile
2299+
from pyiceberg.catalog.rest.scan_planning import RESTDataFile, RESTPositionDeleteFile
22922300

22932301
if isinstance(rest_file, RESTDataFile):
22942302
column_sizes = rest_file.column_sizes.to_dict() if rest_file.column_sizes else None
@@ -2301,6 +2309,17 @@ def _rest_file_to_data_file(rest_file: RESTContentFile) -> DataFile:
23012309
null_value_counts = None
23022310
nan_value_counts = None
23032311

2312+
referenced_data_file = None
2313+
if isinstance(rest_file, RESTPositionDeleteFile):
2314+
referenced_data_file = rest_file.referenced_data_file
2315+
# Infer the DV target from its task only for range reads, preserving whole-Puffin fallback.
2316+
if (
2317+
referenced_data_file is None
2318+
and rest_file.file_format == FileFormat.PUFFIN
2319+
and (rest_file.content_offset is not None or rest_file.content_size_in_bytes is not None)
2320+
):
2321+
referenced_data_file = default_referenced_data_file
2322+
23042323
data_file = DataFile.from_args(
23052324
content=DataFileContent.from_rest_type(rest_file.content),
23062325
file_path=rest_file.file_path,
@@ -2314,6 +2333,9 @@ def _rest_file_to_data_file(rest_file: RESTContentFile) -> DataFile:
23142333
nan_value_counts=nan_value_counts,
23152334
split_offsets=rest_file.split_offsets,
23162335
sort_order_id=rest_file.sort_order_id,
2336+
referenced_data_file=referenced_data_file,
2337+
content_offset=rest_file.content_offset if isinstance(rest_file, RESTPositionDeleteFile) else None,
2338+
content_size_in_bytes=rest_file.content_size_in_bytes if isinstance(rest_file, RESTPositionDeleteFile) else None,
23172339
)
23182340
data_file.spec_id = rest_file.spec_id
23192341
return data_file

‎pyiceberg/table/delete_file_index.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def is_empty(self) -> bool:
115115
def add_delete_file(self, manifest_entry: ManifestEntry, partition_key: Record | None = None) -> None:
116116
delete_file = manifest_entry.data_file
117117
seq = manifest_entry.sequence_number or INITIAL_SEQUENCE_NUMBER
118-
target_path = _referenced_data_file_path(delete_file)
118+
target_path = delete_file.referenced_data_file or _referenced_data_file_path(delete_file)
119119

120120
if target_path:
121121
deletes = self._by_path.setdefault(target_path, PositionDeletes())
@@ -129,6 +129,7 @@ def for_data_file(self, seq_num: int, data_file: DataFile, partition_key: Record
129129
if self.is_empty():
130130
return set()
131131

132+
# DVs are indexed by target path; a valid snapshot has at most one DV per data file.
132133
deletes: set[DataFile] = set()
133134
spec_id = data_file.spec_id or 0
134135

‎pyiceberg/table/deletion_vector.py‎

Lines changed: 100 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717
import math
18+
import struct
19+
import zlib
1820
from typing import TYPE_CHECKING
1921

2022
from pyroaring import BitMap, FrozenBitMap
@@ -24,9 +26,18 @@
2426
if TYPE_CHECKING:
2527
import pyarrow as pa
2628

29+
from pyiceberg.io import FileIO
30+
from pyiceberg.manifest import DataFile
31+
2732
EMPTY_BITMAP = FrozenBitMap()
2833
MAX_JAVA_SIGNED = int(math.pow(2, 31)) - 1
2934
PROPERTY_REFERENCED_DATA_FILE = "referenced-data-file"
35+
_DV_BLOB_LENGTH = struct.Struct(">I")
36+
_DV_BLOB_MAGIC = struct.Struct("<I")
37+
_DV_BLOB_CRC = struct.Struct(">I")
38+
_DV_BLOB_MAGIC_NUMBER = 1681511377
39+
_ROARING_BITMAP_COUNT_SIZE_BYTES = 8
40+
_DV_BLOB_MIN_SIZE_BYTES = _DV_BLOB_LENGTH.size + _DV_BLOB_MAGIC.size + _ROARING_BITMAP_COUNT_SIZE_BYTES + _DV_BLOB_CRC.size
3041

3142

3243
class DeletionVector:
@@ -77,17 +88,101 @@ def to_vector(self) -> "pa.ChunkedArray":
7788
return self._bitmaps_to_chunked_array(self._bitmaps)
7889

7990

80-
def _extract_vector_payload(blob_payload: bytes) -> bytes:
81-
"""Strip deletion-vector-v1 blob framing: length(4 big-endian) + DV magic(4) ... CRC(4 big-endian)."""
82-
length_prefix = int.from_bytes(blob_payload[0:4], "big")
83-
return blob_payload[8 : 4 + length_prefix]
91+
def _deserialize_dv_blob(blob: bytes, record_count: int | None = None) -> list[BitMap]:
92+
# The DV blob encoding matches Iceberg Java's BitmapPositionDeleteIndex:
93+
# 4-byte big-endian bitmap-data length, 4-byte little-endian magic number,
94+
# portable Roaring bitmap data, and 4-byte big-endian CRC-32.
95+
if len(blob) < _DV_BLOB_MIN_SIZE_BYTES:
96+
raise ValueError(f"Invalid deletion vector blob length: {len(blob)}")
97+
98+
bitmap_data_length = _DV_BLOB_LENGTH.unpack_from(blob)[0]
99+
expected_bitmap_data_length = len(blob) - _DV_BLOB_LENGTH.size - _DV_BLOB_CRC.size
100+
if bitmap_data_length != expected_bitmap_data_length:
101+
raise ValueError(f"Invalid bitmap data length: {bitmap_data_length}, expected {expected_bitmap_data_length}")
102+
103+
bitmap_data_offset = _DV_BLOB_LENGTH.size
104+
crc_offset = bitmap_data_offset + bitmap_data_length
105+
bitmap_data = blob[bitmap_data_offset:crc_offset]
106+
107+
magic_number = _DV_BLOB_MAGIC.unpack_from(bitmap_data)[0]
108+
if magic_number != _DV_BLOB_MAGIC_NUMBER:
109+
raise ValueError(f"Invalid magic number: {magic_number}, expected {_DV_BLOB_MAGIC_NUMBER}")
110+
111+
checksum = zlib.crc32(bitmap_data) & 0xFFFFFFFF
112+
expected_checksum = _DV_BLOB_CRC.unpack_from(blob, crc_offset)[0]
113+
if checksum != expected_checksum:
114+
raise ValueError("Invalid CRC")
115+
116+
bitmaps = DeletionVector._deserialize_bitmap(bitmap_data[_DV_BLOB_MAGIC.size :])
117+
if record_count is not None:
118+
cardinality = sum(len(bitmap) for bitmap in bitmaps)
119+
if cardinality != record_count:
120+
raise ValueError(f"Invalid cardinality: {cardinality}, expected {record_count}")
121+
122+
return bitmaps
123+
124+
125+
def _validate_deletion_vector_content(dv: "DataFile") -> tuple[int, int, str]:
126+
content_offset = dv.content_offset
127+
content_size_in_bytes = dv.content_size_in_bytes
128+
referenced_data_file = dv.referenced_data_file
129+
130+
if content_offset is None:
131+
raise ValueError(f"Invalid deletion vector, content offset is missing: {dv.file_path}")
132+
if content_size_in_bytes is None:
133+
raise ValueError(f"Invalid deletion vector, content size is missing: {dv.file_path}")
134+
if content_offset < 0:
135+
raise ValueError(f"Invalid deletion vector, content offset cannot be negative: {content_offset}")
136+
if content_size_in_bytes < 0:
137+
raise ValueError(f"Invalid deletion vector, content size cannot be negative: {content_size_in_bytes}")
138+
if content_size_in_bytes > MAX_JAVA_SIGNED:
139+
raise ValueError(f"Cannot read deletion vector larger than 2GB: {content_size_in_bytes}")
140+
if referenced_data_file is None:
141+
raise ValueError(f"Invalid deletion vector, referenced data file is missing: {dv.file_path}")
142+
143+
return content_offset, content_size_in_bytes, referenced_data_file
144+
145+
146+
def _has_deletion_vector_content_reference(dv: "DataFile") -> bool:
147+
"""Return whether any manifest content-reference field is present.
148+
149+
Partial references must be validated as range reads. Whole-Puffin reads
150+
are only used when all content-reference fields are absent.
151+
"""
152+
return dv.content_offset is not None or dv.content_size_in_bytes is not None or dv.referenced_data_file is not None
153+
154+
155+
def _read_deletion_vector(io: "FileIO", dv: "DataFile") -> DeletionVector:
156+
content_offset, content_size_in_bytes, referenced_data_file = _validate_deletion_vector_content(dv)
157+
158+
with io.new_input(dv.file_path).open() as fi:
159+
fi.seek(content_offset)
160+
payload = fi.read(content_size_in_bytes)
161+
162+
if len(payload) != content_size_in_bytes:
163+
raise ValueError(f"Could not read deletion vector, expected {content_size_in_bytes} bytes, got {len(payload)}")
164+
165+
return DeletionVector(
166+
referenced_data_file=referenced_data_file,
167+
bitmaps=_deserialize_dv_blob(payload, dv.record_count),
168+
)
169+
170+
171+
def read_deletion_vectors(io: "FileIO", dv: "DataFile") -> list[DeletionVector]:
172+
"""Read deletion vectors from a delete file or its manifest content range."""
173+
if _has_deletion_vector_content_reference(dv):
174+
return [_read_deletion_vector(io, dv)]
175+
176+
with io.new_input(dv.file_path).open() as fi:
177+
return deletion_vectors_from_puffin_file(PuffinFile(fi.read()))
84178

85179

86180
def deletion_vectors_from_puffin_file(puffin_file: PuffinFile) -> list[DeletionVector]:
181+
"""Read all deletion vectors stored in a Puffin file."""
87182
return [
88183
DeletionVector(
89184
referenced_data_file=blob.properties[PROPERTY_REFERENCED_DATA_FILE],
90-
bitmaps=DeletionVector._deserialize_bitmap(_extract_vector_payload(puffin_file.get_blob_payload(blob))),
185+
bitmaps=_deserialize_dv_blob(puffin_file.get_blob_payload(blob)),
91186
)
92187
for blob in puffin_file.footer.blobs
93188
]

‎tests/catalog/test_scan_planning_models.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
)
4141
from pyiceberg.expressions import AlwaysTrue, EqualTo, Reference
4242
from pyiceberg.manifest import FileFormat
43+
from pyiceberg.table import FileScanTask
4344

4445
TEST_URI = "https://iceberg-test-catalog/"
4546

@@ -526,6 +527,80 @@ def test_plan_scan_with_delete_files(rest_scan_catalog: RestCatalog, requests_mo
526527
assert len(tasks[0].delete_files) == 1
527528

528529

530+
@pytest.mark.parametrize("include_referenced_data_file", [True, False])
531+
def test_plan_scan_with_shared_dv_file(
532+
rest_scan_catalog: RestCatalog, requests_mock: Mocker, include_referenced_data_file: bool
533+
) -> None:
534+
data_files = [_rest_data_file(file_path=f"s3://bucket/data-{index}.parquet") for index in range(2)]
535+
delete_files = [
536+
{
537+
**_rest_position_delete_file(
538+
file_path="s3://bucket/deletes.bin", file_format="puffin", file_size_in_bytes=400, content_offset=offset
539+
),
540+
**({"referenced-data-file": data_file["file-path"]} if include_referenced_data_file else {}),
541+
}
542+
for data_file, offset in zip(data_files, [0, 200], strict=True)
543+
]
544+
requests_mock.post(
545+
f"{TEST_URI}v1/namespaces/db/tables/tbl/plan",
546+
json={
547+
"status": "completed",
548+
"delete-files": delete_files,
549+
"file-scan-tasks": [
550+
{"data-file": data_file, "delete-file-references": [index, index]} for index, data_file in enumerate(data_files)
551+
],
552+
},
553+
)
554+
555+
tasks = rest_scan_catalog.plan_scan(("db", "tbl"), PlanTableScanRequest())
556+
557+
assert len(tasks) == 2
558+
assert all(len(task.delete_files) == 1 for task in tasks)
559+
deletes = [dv for task in tasks for dv in task.delete_files]
560+
assert len(deletes) == 2
561+
assert {(dv.referenced_data_file, dv.content_offset, dv.content_size_in_bytes) for dv in deletes} == {
562+
("s3://bucket/data-0.parquet", 0, 200),
563+
("s3://bucket/data-1.parquet", 200, 200),
564+
}
565+
566+
567+
@pytest.mark.parametrize("missing_field", ["content-offset", "content-size-in-bytes"])
568+
@pytest.mark.parametrize("include_referenced_data_file", [True, False])
569+
def test_rest_task_preserves_partial_dv_reference(missing_field: str, include_referenced_data_file: bool) -> None:
570+
fields = {
571+
**_rest_position_delete_file(file_format="puffin", content_offset=0),
572+
**({"referenced-data-file": "s3://bucket/data.parquet"} if include_referenced_data_file else {}),
573+
}
574+
fields.pop(missing_field)
575+
task = FileScanTask.from_rest_response(
576+
RESTFileScanTask.model_validate(
577+
{"data-file": _rest_data_file(file_path="s3://bucket/data.parquet"), "delete-file-references": [0]}
578+
),
579+
[RESTPositionDeleteFile.model_validate(fields)],
580+
)
581+
dv = next(iter(task.delete_files))
582+
583+
assert dv.referenced_data_file == "s3://bucket/data.parquet"
584+
assert dv.content_offset == fields.get("content-offset")
585+
assert dv.content_size_in_bytes == fields.get("content-size-in-bytes")
586+
587+
588+
@pytest.mark.parametrize("file_format", ["puffin", "parquet"])
589+
def test_rest_task_preserves_whole_delete_file_reference(file_format: str) -> None:
590+
fields = _rest_position_delete_file(file_format=file_format)
591+
fields.pop("content-offset")
592+
fields.pop("content-size-in-bytes")
593+
task = FileScanTask.from_rest_response(
594+
RESTFileScanTask.model_validate({"data-file": _rest_data_file(), "delete-file-references": [0]}),
595+
[RESTPositionDeleteFile.model_validate(fields)],
596+
)
597+
dv = next(iter(task.delete_files))
598+
599+
assert dv.referenced_data_file is None
600+
assert dv.content_offset is None
601+
assert dv.content_size_in_bytes is None
602+
603+
529604
def test_plan_scan_async_poll_completes(
530605
rest_scan_catalog: RestCatalog, requests_mock: Mocker, monkeypatch: pytest.MonkeyPatch
531606
) -> None:

0 commit comments

Comments
 (0)