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>
PyIceberg is a Python library for programmatic access to Iceberg table metadata as well as to table data in Iceberg format. It is a Python implementation of the Iceberg table spec.
The documentation is available at https://py.iceberg.apache.org/.