Support range-based reads for deletion vectors

# Conflicts:
#	pyiceberg/table/deletion_vector.py

Author: KaiqiJinWow

pyiceberg/io/pyarrow.py

@@ -144,11 +144,10 @@
     visit_with_partner,
 )
 from pyiceberg.table import DOWNCAST_NS_TIMESTAMP_TO_US_ON_WRITE, TableProperties
-from pyiceberg.table.deletion_vector import deletion_vectors_from_puffin_file
+from pyiceberg.table.deletion_vector import read_deletion_vectors
 from pyiceberg.table.locations import load_location_provider
 from pyiceberg.table.metadata import TableMetadata
 from pyiceberg.table.name_mapping import NameMapping, apply_name_mapping
-from pyiceberg.table.puffin import PuffinFile
 from pyiceberg.transforms import IdentityTransform, TruncateTransform
 from pyiceberg.typedef import EMPTY_DICT, Properties, Record, TableVersion
 from pyiceberg.types import (
@@ -1139,10 +1138,7 @@ def _read_deletes(io: FileIO, data_file: DataFile) -> dict[str, pa.ChunkedArray]
                 for path in table.column("file_path").unique()
             }
     elif data_file.file_format == FileFormat.PUFFIN:
-        with io.new_input(data_file.file_path).open() as fi:
-            payload = fi.read()
-
-        return {dv.referenced_data_file: dv.to_vector() for dv in deletion_vectors_from_puffin_file(PuffinFile(payload))}
+        return {dv.referenced_data_file: dv.to_vector() for dv in read_deletion_vectors(io, data_file)}
     else:
         raise ValueError(f"Delete file format not supported: {data_file.file_format}")
 

pyiceberg/manifest.py

@@ -532,6 +532,18 @@ def equality_ids(self) -> list[int] | None:
     def sort_order_id(self) -> int | None:
         return self._data[15]
 
+    @property
+    def referenced_data_file(self) -> str | None:
+        return self._data[17] if len(self._data) > 17 else None
+
+    @property
+    def content_offset(self) -> int | None:
+        return self._data[18] if len(self._data) > 18 else None
+
+    @property
+    def content_size_in_bytes(self) -> int | None:
+        return self._data[19] if len(self._data) > 19 else None
+
     # Spec ID should not be stored in the file
     _spec_id: int
 

pyiceberg/table/deletion_vector.py

@@ -15,7 +15,9 @@
 # specific language governing permissions and limitations
 # under the License.
 import math
-from typing import TYPE_CHECKING
+import struct
+import zlib
+from typing import TYPE_CHECKING, cast
 
 from pyroaring import BitMap, FrozenBitMap
 
@@ -24,9 +26,19 @@
 if TYPE_CHECKING:
     import pyarrow as pa
 
+    from pyiceberg.io import FileIO
+    from pyiceberg.manifest import DataFile
+
 EMPTY_BITMAP = FrozenBitMap()
 MAX_JAVA_SIGNED = int(math.pow(2, 31)) - 1
 PROPERTY_REFERENCED_DATA_FILE = "referenced-data-file"
+_MAX_DELETION_VECTOR_CONTENT_SIZE = 2**31 - 1
+_DV_BLOB_LENGTH = struct.Struct(">I")
+_DV_BLOB_MAGIC = struct.Struct("<I")
+_DV_BLOB_CRC = struct.Struct(">I")
+_DV_BLOB_MAGIC_NUMBER = 1681511377
+_ROARING_BITMAP_COUNT_SIZE_BYTES = 8
+_DV_BLOB_MIN_SIZE_BYTES = _DV_BLOB_LENGTH.size + _DV_BLOB_MAGIC.size + _ROARING_BITMAP_COUNT_SIZE_BYTES + _DV_BLOB_CRC.size
 
 
 class DeletionVector:
@@ -77,17 +89,99 @@ def to_vector(self) -> "pa.ChunkedArray":
         return self._bitmaps_to_chunked_array(self._bitmaps)
 
 
-def _extract_vector_payload(blob_payload: bytes) -> bytes:
-    """Strip deletion-vector-v1 blob framing: length(4 big-endian) + DV magic(4) ... CRC(4 big-endian)."""
-    length_prefix = int.from_bytes(blob_payload[0:4], "big")
-    return blob_payload[8 : 4 + length_prefix]
+def _deserialize_dv_blob(blob: bytes, record_count: int | None = None) -> list[BitMap]:
+    # The DV blob encoding matches Iceberg Java's BitmapPositionDeleteIndex:
+    # 4-byte big-endian bitmap-data length, 4-byte little-endian magic number,
+    # portable Roaring bitmap data, and 4-byte big-endian CRC-32.
+    if len(blob) < _DV_BLOB_MIN_SIZE_BYTES:
+        raise ValueError(f"Invalid deletion vector blob length: {len(blob)}")
+
+    bitmap_data_length = _DV_BLOB_LENGTH.unpack_from(blob)[0]
+    expected_bitmap_data_length = len(blob) - _DV_BLOB_LENGTH.size - _DV_BLOB_CRC.size
+    if bitmap_data_length != expected_bitmap_data_length:
+        raise ValueError(f"Invalid bitmap data length: {bitmap_data_length}, expected {expected_bitmap_data_length}")
+
+    bitmap_data_offset = _DV_BLOB_LENGTH.size
+    crc_offset = bitmap_data_offset + bitmap_data_length
+    bitmap_data = blob[bitmap_data_offset:crc_offset]
+
+    magic_number = _DV_BLOB_MAGIC.unpack_from(bitmap_data)[0]
+    if magic_number != _DV_BLOB_MAGIC_NUMBER:
+        raise ValueError(f"Invalid magic number: {magic_number}, expected {_DV_BLOB_MAGIC_NUMBER}")
+
+    checksum = zlib.crc32(bitmap_data) & 0xFFFFFFFF
+    expected_checksum = _DV_BLOB_CRC.unpack_from(blob, crc_offset)[0]
+    if checksum != expected_checksum:
+        raise ValueError("Invalid CRC")
+
+    bitmaps = DeletionVector._deserialize_bitmap(bitmap_data[_DV_BLOB_MAGIC.size :])
+    if record_count is not None:
+        cardinality = sum(len(bitmap) for bitmap in bitmaps)
+        if cardinality != record_count:
+            raise ValueError(f"Invalid cardinality: {cardinality}, expected {record_count}")
+
+    return bitmaps
+
+
+def _validate_deletion_vector_content(dv: "DataFile") -> None:
+    content_offset = dv.content_offset
+    content_size_in_bytes = dv.content_size_in_bytes
+    referenced_data_file = dv.referenced_data_file
+
+    if content_offset is None:
+        raise ValueError(f"Invalid deletion vector, content offset is missing: {dv.file_path}")
+    if content_size_in_bytes is None:
+        raise ValueError(f"Invalid deletion vector, content size is missing: {dv.file_path}")
+    if content_offset < 0:
+        raise ValueError(f"Invalid deletion vector, content offset cannot be negative: {content_offset}")
+    if content_size_in_bytes < 0:
+        raise ValueError(f"Invalid deletion vector, content size cannot be negative: {content_size_in_bytes}")
+    if content_size_in_bytes > _MAX_DELETION_VECTOR_CONTENT_SIZE:
+        raise ValueError(f"Cannot read deletion vector larger than 2GB: {content_size_in_bytes}")
+    if referenced_data_file is None:
+        raise ValueError(f"Invalid deletion vector, referenced data file is missing: {dv.file_path}")
+
+
+def has_deletion_vector_content_reference(dv: "DataFile") -> bool:
+    """Return whether a deletion vector is described by manifest content-range metadata."""
+    return dv.content_offset is not None or dv.content_size_in_bytes is not None or dv.referenced_data_file is not None
+
+
+def _read_deletion_vector(io: "FileIO", dv: "DataFile") -> DeletionVector:
+    _validate_deletion_vector_content(dv)
+
+    content_offset = cast(int, dv.content_offset)
+    content_size_in_bytes = cast(int, dv.content_size_in_bytes)
+    referenced_data_file = cast(str, dv.referenced_data_file)
+
+    with io.new_input(dv.file_path).open() as fi:
+        fi.seek(content_offset)
+        payload = fi.read(content_size_in_bytes)
+
+    if len(payload) != content_size_in_bytes:
+        raise ValueError(f"Could not read deletion vector, expected {content_size_in_bytes} bytes, got {len(payload)}")
+
+    return DeletionVector(
+        referenced_data_file=referenced_data_file,
+        bitmaps=_deserialize_dv_blob(payload, dv.record_count),
+    )
+
+
+def read_deletion_vectors(io: "FileIO", dv: "DataFile") -> list[DeletionVector]:
+    """Read deletion vectors from a delete file or its manifest content range."""
+    if has_deletion_vector_content_reference(dv):
+        return [_read_deletion_vector(io, dv)]
+
+    with io.new_input(dv.file_path).open() as fi:
+        return deletion_vectors_from_puffin_file(PuffinFile(fi.read()))
 
 
 def deletion_vectors_from_puffin_file(puffin_file: PuffinFile) -> list[DeletionVector]:
+    """Read all deletion vectors stored in a Puffin file."""
     return [
         DeletionVector(
             referenced_data_file=blob.properties[PROPERTY_REFERENCED_DATA_FILE],
-            bitmaps=DeletionVector._deserialize_bitmap(_extract_vector_payload(puffin_file.get_blob_payload(blob))),
+            bitmaps=_deserialize_dv_blob(puffin_file.get_blob_payload(blob)),
         )
         for blob in puffin_file.footer.blobs
     ]

tests/avro/test_file.py

@@ -51,6 +51,7 @@
     LongType,
     NestedField,
     StringType,
+    StructType,
     TimestampType,
     TimestamptzType,
     TimeType,
@@ -89,7 +90,16 @@ def test_missing_schema() -> None:
 
 # helper function to serialize our objects to dicts to enable
 # direct comparison with the dicts returned by fastavro
-def todict(obj: Any) -> Any:
+def todict(obj: Any, struct: StructType | None = None) -> Any:
+    if struct is not None and isinstance(obj, Record):
+        return {
+            field.name: todict(
+                getattr(obj, field.name),
+                field.field_type if isinstance(field.field_type, StructType) else None,
+            )
+            for field in struct.fields
+            if hasattr(obj, field.name)
+        }
     if isinstance(obj, dict):
         data = []
         for k, v in obj.items():
@@ -157,7 +167,7 @@ def test_write_manifest_entry_with_iceberg_read_with_fastavro_v1() -> None:
 
             fa_entry = next(it)
 
-        v2_entry = todict(entry)
+        v2_entry = todict(entry, MANIFEST_ENTRY_SCHEMAS[2].as_struct())
 
         # These are not written in V1
         del v2_entry["sequence_number"]
@@ -222,7 +232,7 @@ def test_write_manifest_entry_with_iceberg_read_with_fastavro_v2() -> None:
 
             fa_entry = next(it)
 
-        assert todict(entry) == fa_entry
+        assert todict(entry, MANIFEST_ENTRY_SCHEMAS[2].as_struct()) == fa_entry
 
 
 @pytest.mark.parametrize("format_version", [1, 2])
@@ -260,7 +270,7 @@ def test_write_manifest_entry_with_fastavro_read_with_iceberg(format_version: Ta
         schema = AvroSchemaConversion().iceberg_to_avro(MANIFEST_ENTRY_SCHEMAS[format_version], schema_name="manifest_entry")
 
         with open(tmp_avro_file, "wb") as out:
-            writer(out, schema, [todict(entry)])
+            writer(out, schema, [todict(entry, MANIFEST_ENTRY_SCHEMAS[format_version].as_struct())])
 
         # Read as V2
         with avro.AvroFile[ManifestEntry](

tests/integration/test_rest_manifest.py

@@ -28,17 +28,28 @@
 from pyiceberg.avro.codecs import AvroCompressionCodec
 from pyiceberg.catalog import Catalog, load_catalog
 from pyiceberg.io.pyarrow import PyArrowFileIO
-from pyiceberg.manifest import DataFile, write_manifest
+from pyiceberg.manifest import MANIFEST_ENTRY_SCHEMAS, DataFile, write_manifest
 from pyiceberg.table import Table
 from pyiceberg.typedef import Record
+from pyiceberg.types import StructType
 from pyiceberg.utils.lazydict import LazyDict
 
 
 # helper function to serialize our objects to dicts to enable
 # direct comparison with the dicts returned by fastavro
-def todict(obj: Any, spec_keys: list[str]) -> Any:
+def todict(obj: Any, spec_keys: list[str], struct: StructType | None = None) -> Any:
     if type(obj) is Record:
         return {key: obj[pos] for key, pos in zip(spec_keys, range(len(obj)), strict=True)}
+    if struct is not None and isinstance(obj, Record):
+        return {
+            field.name: todict(
+                getattr(obj, field.name),
+                spec_keys,
+                field.field_type if isinstance(field.field_type, StructType) else None,
+            )
+            for field in struct.fields
+            if hasattr(obj, field.name)
+        }
     if isinstance(obj, dict) or isinstance(obj, LazyDict):
         data = []
         for k, v in obj.items():
@@ -111,7 +122,11 @@ def test_write_sample_manifest(table_test_all_types: Table, compression: AvroCom
     )
     wrapped_entry_v2 = copy(entry)
     wrapped_entry_v2.data_file = wrapped_data_file_v2_debug
-    wrapped_entry_v2_dict = todict(wrapped_entry_v2, [field.name for field in test_spec.fields])
+    wrapped_entry_v2_dict = todict(
+        wrapped_entry_v2,
+        [field.name for field in test_spec.fields],
+        MANIFEST_ENTRY_SCHEMAS[2].as_struct(),
+    )
 
     with TemporaryDirectory() as tmpdir:
         tmp_avro_file = tmpdir + "/test_write_manifest.avro"