Skip to content

Commit 6cc50cc

Browse files
Write equality_ids as list<int> and keep reading legacy list<long> manifests (#3840)
The Iceberg spec defines data_file.equality_ids as list<int>, but PyIceberg wrote list<long>, which strict readers such as iceberg-cpp reject. Fix the manifest schema so new manifests follow the spec, and let the Avro resolver read the legacy long element as int for that field only, so existing manifests stay readable without loosening type promotion anywhere else. Closes #3840. Co-authored-by: Kevin Liu <kevin.jq.liu@gmail.com>
1 parent 4e6033d commit 6cc50cc

3 files changed

Lines changed: 71 additions & 4 deletions

File tree

pyiceberg/avro/resolver.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,10 +461,25 @@ def primitive(self, primitive: PrimitiveType, expected_primitive: IcebergType |
461461

462462
# ensure that the type can be projected to the expected
463463
if primitive != expected_primitive:
464+
if self._is_legacy_long_equality_ids(primitive, expected_primitive):
465+
return IntegerReader()
464466
promote(primitive, expected_primitive)
465467

466468
return super().primitive(primitive, expected_primitive)
467469

470+
def _is_legacy_long_equality_ids(self, primitive: PrimitiveType, expected_primitive: PrimitiveType) -> bool:
471+
# PyIceberg previously wrote the wrong schema, list<long>, for equality_ids; the Iceberg spec requires list<int>.
472+
# The default schema now uses list<int>, but the schema promotion rules do not allow reading long as int.
473+
# Ints and longs share the same Avro encoding, so allow this exact mismatch for the equality_ids element (136)
474+
# when reading existing files.
475+
# See: https://github.com/apache/iceberg-python/issues/3840
476+
# See: https://iceberg.apache.org/spec/#manifests
477+
return (
478+
self.context == [2, 135, 136] # field id path from manifest_entry: data_file (2), equality_ids (135), element (136)
479+
and isinstance(primitive, LongType)
480+
and isinstance(expected_primitive, IntegerType)
481+
)
482+
468483
def visit_boolean(self, boolean_type: BooleanType, partner: IcebergType | None) -> Reader:
469484
return BooleanReader()
470485

pyiceberg/manifest.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ def __repr__(self) -> str:
295295
NestedField(
296296
field_id=135,
297297
name="equality_ids",
298-
field_type=ListType(element_id=136, element_type=LongType(), element_required=True),
298+
field_type=ListType(element_id=136, element_type=IntegerType(), element_required=True),
299299
required=False,
300300
doc="Field ids used to determine row equality in equality delete files.",
301301
),
@@ -390,7 +390,7 @@ def __repr__(self) -> str:
390390
NestedField(
391391
field_id=135,
392392
name="equality_ids",
393-
field_type=ListType(element_id=136, element_type=LongType(), element_required=True),
393+
field_type=ListType(element_id=136, element_type=IntegerType(), element_required=True),
394394
required=False,
395395
doc="Field ids used to determine row equality in equality delete files.",
396396
),

tests/utils/test_manifest.py

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@
2525

2626
import pyiceberg.manifest as manifest_module
2727
from pyiceberg.avro.codecs import AvroCompressionCodec
28-
from pyiceberg.avro.file import AvroOutputFile
28+
from pyiceberg.avro.file import AvroFile, AvroOutputFile
2929
from pyiceberg.io import load_file_io
3030
from pyiceberg.io.pyarrow import PyArrowFileIO
3131
from pyiceberg.manifest import (
32+
DATA_FILE_TYPE,
3233
MANIFEST_ENTRY_SCHEMAS,
3334
MANIFEST_LIST_FILE_SCHEMAS,
3435
DataFile,
@@ -50,7 +51,7 @@
5051
from pyiceberg.schema import Schema
5152
from pyiceberg.table.snapshots import Operation, Snapshot, Summary
5253
from pyiceberg.typedef import Record, TableVersion
53-
from pyiceberg.types import IntegerType, NestedField
54+
from pyiceberg.types import IntegerType, ListType, LongType, NestedField, StructType
5455

5556

5657
@pytest.fixture(autouse=True)
@@ -294,6 +295,57 @@ def write_and_read(file_name: str, data_file: DataFile) -> DataFile:
294295
assert delete_file.content_size_in_bytes == 46
295296

296297

298+
def test_read_legacy_long_equality_ids(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
299+
"""Manifests written by older PyIceberg versions with equality_ids as list<long> can still be read.
300+
301+
PyIceberg previously wrote the wrong schema, list<long>, for equality_ids; the Iceberg spec requires list<int>.
302+
The default schema now uses list<int>, so reading existing manifests relies on the fallback in the Avro resolver.
303+
See: https://github.com/apache/iceberg-python/issues/3840
304+
See: https://iceberg.apache.org/spec/#manifests
305+
"""
306+
io = PyArrowFileIO()
307+
manifest_path = str(tmp_path / "manifest.avro")
308+
309+
entry = ManifestEntry.from_args(
310+
status=ManifestEntryStatus.ADDED,
311+
snapshot_id=1,
312+
sequence_number=1,
313+
file_sequence_number=1,
314+
data_file=DataFile.from_args(
315+
content=DataFileContent.EQUALITY_DELETES,
316+
file_path="s3://bucket/deletes.parquet",
317+
file_format=FileFormat.PARQUET,
318+
partition=Record(),
319+
record_count=10,
320+
file_size_in_bytes=1024,
321+
equality_ids=[1, 2],
322+
),
323+
)
324+
325+
# Write the manifest as older PyIceberg versions did, with equality_ids as list<long>
326+
legacy_data_file_type = StructType(
327+
*[
328+
NestedField(135, "equality_ids", ListType(136, LongType()), required=False) if field.field_id == 135 else field
329+
for field in DATA_FILE_TYPE[2].fields
330+
]
331+
)
332+
with monkeypatch.context() as legacy:
333+
legacy.setitem(DATA_FILE_TYPE, 2, legacy_data_file_type)
334+
with write_manifest(
335+
format_version=2,
336+
spec=UNPARTITIONED_PARTITION_SPEC,
337+
schema=Schema(NestedField(1, "foo", IntegerType(), False)),
338+
output_file=io.new_output(manifest_path),
339+
snapshot_id=1,
340+
avro_compression="null",
341+
) as writer:
342+
writer.add_entry(entry)
343+
with AvroFile[ManifestEntry](io.new_input(manifest_path)) as avro_file:
344+
assert avro_file.schema.find_field("data_file.equality_ids").field_type == ListType(136, LongType())
345+
346+
assert writer.to_manifest_file().fetch_manifest_entry(io)[0].data_file.equality_ids == [1, 2]
347+
348+
297349
def test_read_manifest_list(generated_manifest_file_file_v1: str) -> None:
298350
input_file = PyArrowFileIO().new_input(generated_manifest_file_file_v1)
299351
manifest_list = list(read_manifest_list(input_file))[0]

0 commit comments

Comments
 (0)