fix(validate): report every conflicting snapshot in validation errors

pyiceberg/table/update/validate.py

@@ -190,8 +190,8 @@ def _validate_deleted_data_files(
         parent_snapshot: Ending snapshot on the branch being validated
 
     """
-    conflicting_entries = _deleted_data_files(table, starting_snapshot, data_filter, None, parent_snapshot)
-    if any(conflicting_entries):
+    conflicting_entries = list(_deleted_data_files(table, starting_snapshot, data_filter, None, parent_snapshot))
+    if conflicting_entries:
         conflicting_snapshots = {entry.snapshot_id for entry in conflicting_entries}
         raise ValidationException(f"Deleted data files were found matching the filter for snapshots {conflicting_snapshots}!")
 
@@ -325,8 +325,8 @@ def _validate_added_data_files(
         parent_snapshot: Ending snapshot on the branch being validated
 
     """
-    conflicting_entries = _added_data_files(table, starting_snapshot, data_filter, None, parent_snapshot)
-    if any(conflicting_entries):
+    conflicting_entries = list(_added_data_files(table, starting_snapshot, data_filter, None, parent_snapshot))
+    if conflicting_entries:
         conflicting_snapshots = {entry.snapshot_id for entry in conflicting_entries if entry.snapshot_id is not None}
         raise ValidationException(f"Added data files were found matching the filter for snapshots {conflicting_snapshots}!")
 

tests/table/test_validate.py

@@ -363,6 +363,68 @@ class DummyEntry:
             )
 
 
+def test_validate_added_data_files_reports_every_conflicting_snapshot(
+    table_v2_with_extensive_snapshots_and_manifests: tuple[Table, dict[int, list[ManifestFile]]],
+) -> None:
+    """The helpers return iterators, so the entries must survive the truthiness check.
+
+    `any(entries)` consumes the iterator up to the first truthy element, leaving the
+    set comprehension that builds the error message to read what is left. With a
+    single conflicting entry that produced an empty set, i.e. a ValidationException
+    that could not name the snapshot it conflicted with.
+    """
+    table, _ = table_v2_with_extensive_snapshots_and_manifests
+    oldest_snapshot = table.snapshots()[0]
+    newest_snapshot = cast(Snapshot, table.current_snapshot())
+
+    class DummyEntry:
+        def __init__(self, snapshot_id: int) -> None:
+            self.snapshot_id = snapshot_id
+
+    for snapshot_ids in ([123], [123, 456, 789]):
+        with patch(
+            "pyiceberg.table.update.validate._added_data_files",
+            return_value=iter([DummyEntry(i) for i in snapshot_ids]),
+        ):
+            with pytest.raises(ValidationException) as exc_info:
+                _validate_added_data_files(
+                    table=table,
+                    starting_snapshot=newest_snapshot,
+                    data_filter=None,
+                    parent_snapshot=oldest_snapshot,
+                )
+        message = str(exc_info.value)
+        for snapshot_id in snapshot_ids:
+            assert str(snapshot_id) in message, f"{snapshot_id} missing from {message!r}"
+
+
+def test_validate_deleted_data_files_reports_every_conflicting_snapshot(
+    table_v2_with_extensive_snapshots_and_manifests: tuple[Table, dict[int, list[ManifestFile]]],
+) -> None:
+    """Same iterator-consumption problem in the deleted-files validator."""
+    table, _ = table_v2_with_extensive_snapshots_and_manifests
+    oldest_snapshot = table.snapshots()[0]
+    newest_snapshot = cast(Snapshot, table.current_snapshot())
+
+    class DummyEntry:
+        def __init__(self, snapshot_id: int) -> None:
+            self.snapshot_id = snapshot_id
+
+    with patch(
+        "pyiceberg.table.update.validate._deleted_data_files",
+        return_value=iter([DummyEntry(123)]),
+    ):
+        with pytest.raises(ValidationException) as exc_info:
+            _validate_deleted_data_files(
+                table=table,
+                starting_snapshot=newest_snapshot,
+                data_filter=None,
+                parent_snapshot=oldest_snapshot,
+            )
+
+    assert "123" in str(exc_info.value)
+
+
 @pytest.mark.parametrize("operation", [Operation.APPEND, Operation.REPLACE])
 def test_added_delete_files_non_conflicting_count(
     table_v2_with_extensive_snapshots_and_manifests: tuple[Table, dict[int, list[ManifestFile]]],