fix: preserve manifest min sequence number of 0 (#3660)

# Rationale for this change

`ManifestWriter.to_manifest_file()` computed the manifest's minimum data
sequence number with a truthiness fallback:

```python
min_sequence_number = self._min_sequence_number or UNASSIGNED_SEQ
```

A data sequence number of `0` is a legitimate minimum for a live file
(files
from a v1 table, or the initial commit of a v2 table). Because `0` is
falsy,
`or` collapses it to `UNASSIGNED_SEQ` (`-1`), i.e. it treats a real `0`
the same
as "unset".

This diverges from the Java reference implementation, which falls back
to
`UNASSIGNED_SEQ` only when the value is actually unset:

```java
// ManifestWriter#toManifestFile (apache/iceberg)
long minSeqNumber = minDataSequenceNumber != null ? minDataSequenceNumber : UNASSIGNED_SEQ;
```

The `-1` is not harmless. When the manifest is produced by a
merge/compaction
(so its `added_snapshot_id` equals the current commit snapshot id),
`ManifestListWriter.prepare_manifest()` treats `min_sequence_number ==
UNASSIGNED_SEQ` as "no file had an assigned sequence number" and
overwrites it
with the current, higher commit sequence number
(`pyiceberg/manifest.py`, the `if
wrapped_manifest_file.min_sequence_number ==
UNASSIGNED_SEQ:` branch). The manifest's minimum data sequence number is
thereby
silently raised, which affects sequence-number-based delete-file
application and
scans.

The fix uses an explicit `None` check instead of a truthiness fallback,
mirroring the Java writer:

```python
min_sequence_number = self._min_sequence_number if self._min_sequence_number is not None else UNASSIGNED_SEQ
```

## Are these changes tested?

Yes. A new parametrized regression test,

`tests/utils/test_manifest.py::test_write_manifest_min_sequence_number_zero`
(format versions 1 and 2), drives a live `EXISTING` entry with
`sequence_number=0` through `ManifestWriter.existing()` and asserts that
`to_manifest_file().min_sequence_number == 0`.

The test fails before the fix (`assert -1 == 0`) and passes after it.
This path
had no prior coverage: no existing test drove a live sequence-0 entry
through
`to_manifest_file()`, which is why the defect was not caught.

Local runs (unit tests):
- `pytest tests/utils/test_manifest.py -q` -> passes.
- `pytest tests/table/test_snapshots.py
tests/table/test_manage_snapshots.py -q`
  -> passes.
- `make lint` (ruff, ruff-format, mypy, pydocstyle, codespell) -> clean
on the
  changed files.

The merge/compaction integration path exercised by `prepare_manifest()`
is
covered by the integration suite, which requires Docker and Spark; those
were
not run locally. The new unit test exercises the same
`to_manifest_file()` write
path that feeds it.

## Are there any user-facing changes?

No API changes. It is a correctness fix: manifests written for live
files whose
minimum data sequence number is `0` now record `0` instead of `-1`, so a
subsequent merge/compaction no longer silently raises the manifest's
minimum
data sequence number.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>

Author: anxkhn

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),

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,