Pass first-row-id to the V3 manifest list writer

Author: moomindani

pyiceberg/table/update/snapshot.py

@@ -328,21 +328,22 @@ def _commit(self) -> UpdatesAndRequirements:
         manifest_list_file_path = location_provider.new_metadata_location(file_name)
         self._written_manifest_lists.append(manifest_list_file_path)
 
+        first_row_id: int | None = None
+
+        if self._transaction.table_metadata.format_version >= 3:
+            first_row_id = self._transaction.table_metadata.next_row_id
+
         with write_manifest_list(
             format_version=self._transaction.table_metadata.format_version,
             output_file=self._io.new_output(manifest_list_file_path),
             snapshot_id=self._snapshot_id,
             parent_snapshot_id=self._parent_snapshot_id,
             sequence_number=next_sequence_number,
             avro_compression=self._compression,
+            first_row_id=first_row_id,
         ) as writer:
             writer.add_manifests(new_manifests)
 
-        first_row_id: int | None = None
-
-        if self._transaction.table_metadata.format_version >= 3:
-            first_row_id = self._transaction.table_metadata.next_row_id
-
         snapshot = Snapshot(
             snapshot_id=self._snapshot_id,
             parent_snapshot_id=self._parent_snapshot_id,

tests/table/test_snapshots.py

@@ -737,3 +737,40 @@ def test_overwrite_rejects_explicit_delete_without_parent_snapshot(
         with empty.transaction() as tx:
             with tx.update_snapshot().overwrite() as overwrite:
                 overwrite.delete_data_file(stale_file)
+
+
+def test_manifest_list_receives_first_row_id_for_v3(table_v3: Table) -> None:
+    """The V3 manifest list writer requires first-row-id, so the producer has to supply it."""
+    from unittest import mock
+
+    from pyiceberg.table.update.snapshot import _FastAppendFiles
+
+    txn = table_v3.transaction()
+    append = _FastAppendFiles(operation=Operation.APPEND, transaction=txn, io=table_v3.io)
+
+    with (
+        mock.patch("pyiceberg.table.update.snapshot.write_manifest_list") as writer,
+        mock.patch.object(_FastAppendFiles, "_manifests", return_value=[]),
+    ):
+        append._commit()
+
+    assert writer.call_args.kwargs["format_version"] == 3
+    assert writer.call_args.kwargs["first_row_id"] == table_v3.metadata.next_row_id
+
+
+def test_manifest_list_has_no_first_row_id_for_v2(table_v2: Table) -> None:
+    from unittest import mock
+
+    from pyiceberg.table.update.snapshot import _FastAppendFiles
+
+    txn = table_v2.transaction()
+    append = _FastAppendFiles(operation=Operation.APPEND, transaction=txn, io=table_v2.io)
+
+    with (
+        mock.patch("pyiceberg.table.update.snapshot.write_manifest_list") as writer,
+        mock.patch.object(_FastAppendFiles, "_manifests", return_value=[]),
+    ):
+        append._commit()
+
+    assert writer.call_args.kwargs["format_version"] == 2
+    assert writer.call_args.kwargs["first_row_id"] is None