From 9af3f57633b58bdc7e9db90ebb6dc8fd8dbb45c4 Mon Sep 17 00:00:00 2001 From: Anas Khan <83116240+anxkhn@users.noreply.github.com> Date: Sat, 4 Jul 2026 22:44:54 +0530 Subject: [PATCH] fix: preserve manifest min sequence number of 0 ManifestWriter.to_manifest_file() used `self._min_sequence_number or UNASSIGNED_SEQ`, which treats a legitimate minimum data sequence number of 0 as falsy and collapses it to UNASSIGNED_SEQ (-1). A data sequence number of 0 is valid for a live file (files from a v1 table, or the initial commit of a v2 table), so this diverges from the Java reference, which falls back to UNASSIGNED_SEQ only when the minimum is unset (null). When such a manifest is produced by a merge/compaction (the added snapshot id equals the current commit snapshot id), prepare_manifest() interprets the -1 as "no file had an assigned sequence number" and overwrites it with the current, higher commit sequence number, silently raising the manifest's min data sequence number. That in turn affects sequence-number-based delete-file application and scans. Use an explicit None check, mirroring the Java ManifestWriter, and add a regression test that drives a live existing entry with sequence number 0 through to_manifest_file() for both format versions. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com> --- pyiceberg/manifest.py | 4 +++- tests/utils/test_manifest.py | 42 ++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/pyiceberg/manifest.py b/pyiceberg/manifest.py index 9842f79d8e..37dbd04b13 100644 --- a/pyiceberg/manifest.py +++ b/pyiceberg/manifest.py @@ -1145,7 +1145,9 @@ def to_manifest_file(self) -> ManifestFile: """Return the manifest file.""" # once the manifest file is generated, no more entries can be added self.closed = True - min_sequence_number = self._min_sequence_number or UNASSIGNED_SEQ + # A min sequence number of 0 is legitimate (e.g. live files from a v1 table or the + # initial commit of a v2 table), so fall back to UNASSIGNED_SEQ only when it is unset. + min_sequence_number = self._min_sequence_number if self._min_sequence_number is not None else UNASSIGNED_SEQ return ManifestFile.from_args( manifest_path=self._output_file.location, manifest_length=len(self._writer.output_file), diff --git a/tests/utils/test_manifest.py b/tests/utils/test_manifest.py index f2ae1e05ad..eac4d520bc 100644 --- a/tests/utils/test_manifest.py +++ b/tests/utils/test_manifest.py @@ -938,6 +938,48 @@ def test_manifest_writer_tell(format_version: TableVersion) -> None: assert after_entry_bytes > initial_bytes, "Bytes should increase after adding entry" +@pytest.mark.parametrize("format_version", [1, 2]) +def test_write_manifest_min_sequence_number_zero(format_version: TableVersion) -> None: + # A data sequence number of 0 is a legitimate min for a live file (e.g. files from a + # v1 table or the initial commit of a v2 table). It must be preserved in the manifest, + # not collapsed to UNASSIGNED_SEQ (-1), which would let a merge/compaction silently + # raise the manifest's min data sequence number. This mirrors the Java reference, which + # only falls back to UNASSIGNED_SEQ when the min is unset (null). + io = load_file_io() + test_schema = Schema(NestedField(1, "foo", IntegerType(), False)) + + with TemporaryDirectory() as tmpdir: + output_file = io.new_output(f"{tmpdir}/test-manifest.avro") + with write_manifest( + format_version=format_version, + spec=UNPARTITIONED_PARTITION_SPEC, + schema=test_schema, + output_file=output_file, + snapshot_id=1, + avro_compression="null", + ) as writer: + data_file = DataFile.from_args( + content=DataFileContent.DATA, + file_path=f"{tmpdir}/data.parquet", + file_format=FileFormat.PARQUET, + partition=Record(), + record_count=100, + file_size_in_bytes=1000, + ) + writer.existing( + ManifestEntry.from_args( + status=ManifestEntryStatus.EXISTING, + snapshot_id=1, + sequence_number=0, + file_sequence_number=0, + data_file=data_file, + ) + ) + manifest_file = writer.to_manifest_file() + + assert manifest_file.min_sequence_number == 0 + + def test_inherit_from_manifest_snapshot_id() -> None: entry = ManifestEntry.from_args( status=ManifestEntryStatus.ADDED,