kevinjqliu commented on code in PR #2951:
URL: https://github.com/apache/iceberg-python/pull/2951#discussion_r2725930634
##########
tests/utils/test_manifest.py:
##########
@@ -629,3 +639,268 @@ def test_file_format_case_insensitive(raw_file_format:
str, expected_file_format
else:
with pytest.raises(ValueError):
_ = FileFormat(raw_file_format)
+
+
+def test_manifest_cache_deduplicates_manifest_files() -> None:
+ """Test that the manifest cache deduplicates ManifestFile objects across
manifest lists.
+
+ This test verifies the fix for
https://github.com/apache/iceberg-python/issues/2325
+
+ The issue was that when caching manifest lists by their path, overlapping
ManifestFile
+ objects were duplicated. For example:
+ - ManifestList1: (ManifestFile1)
+ - ManifestList2: (ManifestFile1, ManifestFile2)
+ - ManifestList3: (ManifestFile1, ManifestFile2, ManifestFile3)
+
+ With the old approach, ManifestFile1 was stored 3 times in the cache.
+ With the new approach, ManifestFile objects are cached individually by
their
+ manifest_path, so ManifestFile1 is stored only once and reused.
+ """
+ io = PyArrowFileIO()
+
+ with TemporaryDirectory() as tmp_dir:
+ # Create three manifest files to simulate manifests created during
appends
+ manifest1_path = f"{tmp_dir}/manifest1.avro"
+ manifest2_path = f"{tmp_dir}/manifest2.avro"
+ manifest3_path = f"{tmp_dir}/manifest3.avro"
+
+ schema = Schema(NestedField(field_id=1, name="id",
field_type=IntegerType(), required=True))
+ spec = UNPARTITIONED_PARTITION_SPEC
+
+ # Create manifest file 1
+ with write_manifest(
+ format_version=2,
+ spec=spec,
+ schema=schema,
+ output_file=io.new_output(manifest1_path),
+ snapshot_id=1,
+ avro_compression="zstandard",
+ ) as writer:
+ data_file1 = DataFile.from_args(
+ content=DataFileContent.DATA,
+ file_path=f"{tmp_dir}/data1.parquet",
+ file_format=FileFormat.PARQUET,
+ partition=Record(),
+ record_count=100,
+ file_size_in_bytes=1000,
+ )
+ writer.add_entry(
+ ManifestEntry.from_args(
+ status=ManifestEntryStatus.ADDED,
+ snapshot_id=1,
+ data_file=data_file1,
+ )
+ )
+ manifest_file1 = writer.to_manifest_file()
+
+ # Create manifest file 2
+ with write_manifest(
+ format_version=2,
+ spec=spec,
+ schema=schema,
+ output_file=io.new_output(manifest2_path),
+ snapshot_id=2,
+ avro_compression="zstandard",
+ ) as writer:
+ data_file2 = DataFile.from_args(
+ content=DataFileContent.DATA,
+ file_path=f"{tmp_dir}/data2.parquet",
+ file_format=FileFormat.PARQUET,
+ partition=Record(),
+ record_count=200,
+ file_size_in_bytes=2000,
+ )
+ writer.add_entry(
+ ManifestEntry.from_args(
+ status=ManifestEntryStatus.ADDED,
+ snapshot_id=2,
+ data_file=data_file2,
+ )
+ )
+ manifest_file2 = writer.to_manifest_file()
+
+ # Create manifest file 3
+ with write_manifest(
+ format_version=2,
+ spec=spec,
+ schema=schema,
+ output_file=io.new_output(manifest3_path),
+ snapshot_id=3,
+ avro_compression="zstandard",
+ ) as writer:
+ data_file3 = DataFile.from_args(
+ content=DataFileContent.DATA,
+ file_path=f"{tmp_dir}/data3.parquet",
+ file_format=FileFormat.PARQUET,
+ partition=Record(),
+ record_count=300,
+ file_size_in_bytes=3000,
+ )
+ writer.add_entry(
+ ManifestEntry.from_args(
+ status=ManifestEntryStatus.ADDED,
+ snapshot_id=3,
+ data_file=data_file3,
+ )
+ )
+ manifest_file3 = writer.to_manifest_file()
+
+ # Create manifest list 1: contains only manifest1
+ manifest_list1_path = f"{tmp_dir}/manifest-list1.avro"
+ with write_manifest_list(
+ format_version=2,
+ output_file=io.new_output(manifest_list1_path),
+ snapshot_id=1,
+ parent_snapshot_id=None,
+ sequence_number=1,
+ avro_compression="zstandard",
+ ) as list_writer:
+ list_writer.add_manifests([manifest_file1])
+
+ # Create manifest list 2: contains manifest1 and manifest2
(overlapping manifest1)
+ manifest_list2_path = f"{tmp_dir}/manifest-list2.avro"
+ with write_manifest_list(
+ format_version=2,
+ output_file=io.new_output(manifest_list2_path),
+ snapshot_id=2,
+ parent_snapshot_id=1,
+ sequence_number=2,
+ avro_compression="zstandard",
+ ) as list_writer:
+ list_writer.add_manifests([manifest_file1, manifest_file2])
+
Review Comment:
ignoring since this is in tests, we're only tests manifest cache behavior
--
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]
