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>
2 files changed
tree: e3457c608031fc471a25a20777a7a7e8be2c26ee
  1. .github/
  2. dev/
  3. mkdocs/
  4. notebooks/
  5. pyiceberg/
  6. tests/
  7. vendor/
  8. .asf.yaml
  9. .codespellrc
  10. .gitignore
  11. .markdownlint.yaml
  12. .pre-commit-config.yaml
  13. AGENTS.md
  14. LICENSE
  15. Makefile
  16. MANIFEST.in
  17. NOTICE
  18. pyproject.toml
  19. README.md
  20. ruff.toml
  21. SECURITY-THREAT-MODEL.md
  22. setup.py
  23. uv.lock
README.md

Iceberg Python

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/.

Get in Touch