Skip to content

Commit 992d0ce

Browse files
kevinjqliuCopilot
andcommitted
Use canonical manifest records in Avro tests
Project records to versioned writer schemas by Iceberg field ID so DataFile keeps one stable in-memory layout while v2 field ordering remains schema-correct. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 1ef3c24 commit 992d0ce

4 files changed

Lines changed: 37 additions & 50 deletions

File tree

‎pyiceberg/manifest.py‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -541,21 +541,19 @@ def sort_order_id(self) -> int | None:
541541

542542
@property
543543
def first_row_id(self) -> int | None:
544-
return self._data[16] if len(self._data) >= len(DATA_FILE_TYPE[3].fields) else None
544+
return self._data[16]
545545

546546
@property
547547
def referenced_data_file(self) -> str | None:
548-
if len(self._data) == len(DATA_FILE_TYPE[2].fields):
549-
return self._data[16]
550-
return self._data[17] if len(self._data) >= len(DATA_FILE_TYPE[3].fields) else None
548+
return self._data[17]
551549

552550
@property
553551
def content_offset(self) -> int | None:
554-
return self._data[18] if len(self._data) >= len(DATA_FILE_TYPE[3].fields) else None
552+
return self._data[18]
555553

556554
@property
557555
def content_size_in_bytes(self) -> int | None:
558-
return self._data[19] if len(self._data) >= len(DATA_FILE_TYPE[3].fields) else None
556+
return self._data[19]
559557

560558
# Spec ID should not be stored in the file
561559
_spec_id: int

‎tests/avro/test_file.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ def test_write_manifest_entry_with_iceberg_read_with_fastavro_v1() -> None:
155155

156156
def test_write_v2_manifest_entry_with_fastavro() -> None:
157157
data_file = DataFile.from_args(
158-
_table_format_version=2,
159158
content=DataFileContent.POSITION_DELETES,
160159
file_path="s3://some-path/delete-file.parquet",
161160
file_format=FileFormat.PARQUET,
@@ -174,13 +173,8 @@ def test_write_v2_manifest_entry_with_fastavro() -> None:
174173
sort_order_id=4,
175174
referenced_data_file="s3://some-path/data-file.parquet",
176175
)
177-
assert data_file.first_row_id is None
178-
assert data_file.referenced_data_file == "s3://some-path/data-file.parquet"
179-
assert data_file.content_offset is None
180-
assert data_file.content_size_in_bytes is None
181176

182177
entry = ManifestEntry.from_args(
183-
_table_format_version=2,
184178
status=ManifestEntryStatus.ADDED,
185179
snapshot_id=8638475580105682862,
186180
sequence_number=0,
@@ -197,6 +191,7 @@ def test_write_v2_manifest_entry_with_fastavro() -> None:
197191
output_file=PyArrowFileIO().new_output(tmp_avro_file),
198192
file_schema=MANIFEST_ENTRY_SCHEMAS[2],
199193
schema_name="manifest_entry",
194+
record_schema=MANIFEST_ENTRY_SCHEMAS[3],
200195
metadata=additional_metadata,
201196
) as out:
202197
out.write_block([entry])
@@ -212,7 +207,15 @@ def test_write_v2_manifest_entry_with_fastavro() -> None:
212207

213208
fa_entry = next(it)
214209

215-
assert record_to_fastavro(entry, MANIFEST_ENTRY_SCHEMAS[2].as_struct()) == fa_entry
210+
assert fa_entry["data_file"]["referenced_data_file"] == data_file.referenced_data_file
211+
assert (
212+
record_to_fastavro(
213+
entry,
214+
record_struct=MANIFEST_ENTRY_SCHEMAS[3].as_struct(),
215+
file_struct=MANIFEST_ENTRY_SCHEMAS[2].as_struct(),
216+
)
217+
== fa_entry
218+
)
216219

217220

218221
@pytest.mark.parametrize("format_version", [1, 2])

‎tests/conftest.py‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,28 @@
106106
from pyiceberg.io.pyarrow import PyArrowFileIO
107107

108108

109-
def record_to_fastavro(obj: Any, struct: StructType | None = None) -> Any:
109+
def record_to_fastavro(
110+
obj: Any,
111+
record_struct: StructType | None = None,
112+
file_struct: StructType | None = None,
113+
) -> Any:
110114
"""Convert a manifest record to the representation returned by FastAvro."""
111115
if isinstance(obj, Record):
112-
if struct is not None:
113-
return {
114-
field.name: record_to_fastavro(
116+
if record_struct is not None:
117+
record_positions = {field.field_id: pos for pos, field in enumerate(record_struct.fields)}
118+
result = {}
119+
for file_field in (file_struct or record_struct).fields:
120+
if (pos := record_positions.get(file_field.field_id)) is None:
121+
result[file_field.name] = file_field.write_default
122+
continue
123+
124+
record_field = record_struct.fields[pos]
125+
result[file_field.name] = record_to_fastavro(
115126
obj[pos],
116-
field.field_type if isinstance(field.field_type, StructType) else None,
127+
record_field.field_type if isinstance(record_field.field_type, StructType) else None,
128+
file_field.field_type if isinstance(file_field.field_type, StructType) else None,
117129
)
118-
for pos, field in enumerate(struct.fields)
119-
}
130+
return result
120131
return {
121132
key: record_to_fastavro(value)
122133
for key, value in inspect.getmembers(obj)

‎tests/integration/test_rest_manifest.py‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
from pyiceberg.catalog import Catalog, load_catalog
2727
from pyiceberg.io.pyarrow import PyArrowFileIO
2828
from pyiceberg.manifest import (
29-
DataFile,
30-
ManifestEntry,
3129
data_file_with_partition,
3230
manifest_entry_schema_with_data_file,
3331
write_manifest,
@@ -66,38 +64,15 @@ def test_write_sample_manifest(table_test_all_types: Table, compression: AvroCom
6664
entry = test_manifest_entries[0]
6765
test_schema = table_test_all_types.schema()
6866
test_spec = table_test_all_types.spec()
69-
data_file_v2 = DataFile.from_args(
70-
_table_format_version=2,
71-
content=entry.data_file.content,
72-
file_path=entry.data_file.file_path,
73-
file_format=entry.data_file.file_format,
74-
partition=entry.data_file.partition,
75-
record_count=entry.data_file.record_count,
76-
file_size_in_bytes=entry.data_file.file_size_in_bytes,
77-
column_sizes=entry.data_file.column_sizes,
78-
value_counts=entry.data_file.value_counts,
79-
null_value_counts=entry.data_file.null_value_counts,
80-
nan_value_counts=entry.data_file.nan_value_counts,
81-
lower_bounds=entry.data_file.lower_bounds,
82-
upper_bounds=entry.data_file.upper_bounds,
83-
key_metadata=entry.data_file.key_metadata,
84-
split_offsets=entry.data_file.split_offsets,
85-
equality_ids=entry.data_file.equality_ids,
86-
sort_order_id=entry.data_file.sort_order_id,
87-
referenced_data_file=entry.data_file.referenced_data_file,
88-
)
89-
entry_v2 = ManifestEntry.from_args(
90-
_table_format_version=2,
91-
status=entry.status,
92-
snapshot_id=entry.snapshot_id,
93-
sequence_number=entry.sequence_number,
94-
file_sequence_number=entry.file_sequence_number,
95-
data_file=data_file_v2,
67+
data_file_v3_type = data_file_with_partition(
68+
partition_type=test_spec.partition_type(test_schema),
69+
format_version=3,
9670
)
9771
data_file_v2_type = data_file_with_partition(
9872
partition_type=test_spec.partition_type(test_schema),
9973
format_version=2,
10074
)
75+
entry_v3_type = manifest_entry_schema_with_data_file(format_version=3, data_file=data_file_v3_type).as_struct()
10176
entry_v2_type = manifest_entry_schema_with_data_file(format_version=2, data_file=data_file_v2_type).as_struct()
10277

10378
with TemporaryDirectory() as tmpdir:
@@ -119,4 +94,4 @@ def test_write_sample_manifest(table_test_all_types: Table, compression: AvroCom
11994
it = iter(r)
12095
fa_entry = next(it)
12196

122-
assert fa_entry == record_to_fastavro(entry_v2, entry_v2_type)
97+
assert fa_entry == record_to_fastavro(entry, record_struct=entry_v3_type, file_struct=entry_v2_type)

0 commit comments

Comments
 (0)