kevinjqliu commented on code in PR #3953:
URL: https://github.com/apache/iceberg-python/pull/3953#discussion_r3997076081
##########
tests/avro/test_file.py:
##########
@@ -225,12 +226,44 @@ def
test_write_manifest_entry_with_iceberg_read_with_fastavro_v2() -> None:
fa_entry = next(it)
v2_entry = todict(entry)
- for field in ("first_row_id", "referenced_data_file",
"content_offset", "content_size_in_bytes"):
+ for field in ("first_row_id", "content_offset",
"content_size_in_bytes"):
del v2_entry["data_file"][field]
assert v2_entry == fa_entry
+def test_write_v2_referenced_data_file_with_fastavro() -> None:
+ referenced_data_file = "s3://some-path/data-file.parquet"
+ entry = ManifestEntry.from_args(
+ status=ManifestEntryStatus.ADDED,
+ snapshot_id=25,
+ data_file=DataFile.from_args(
+ content=DataFileContent.POSITION_DELETES,
+ file_path="s3://some-path/delete-file.parquet",
+ file_format=FileFormat.PARQUET,
+ partition=Record(),
+ record_count=3,
+ file_size_in_bytes=47,
+ referenced_data_file=referenced_data_file,
+ ),
+ )
+
+ with TemporaryDirectory() as tmpdir:
+ tmp_avro_file = tmpdir + "/manifest_entry.avro"
+ with avro.AvroOutputFile[ManifestEntry](
+ output_file=PyArrowFileIO().new_output(tmp_avro_file),
+ file_schema=MANIFEST_ENTRY_SCHEMAS[2],
+ record_schema=MANIFEST_ENTRY_SCHEMAS[3],
Review Comment:
We need to declare read as v3 schema here, even though the write schema is
v2. Otherwise, `referenced_data_file` getter will reference the wrong field. ðŸ˜
this is because the DataFile getters use position-based index.
https://github.com/apache/iceberg-python/blob/562d3af3a77476bcd7b923f31da60200c1b2563c/pyiceberg/manifest.py#L539-L541
In V2 schema, `referenced_data_file` should be index 16 because
`first_row_id` doesnt exist in V2.
In V3 schema, `referenced_data_file` is index 17.
The current getter use index 17 and I dont want to add a if branch to the
getter.
This is an existing problem so I think the best way to resolve is to move
away from position index and use field id based projection.
We can do this as a follow up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
