diff --git a/pyiceberg/table/update/validate.py b/pyiceberg/table/update/validate.py index 0545182bf0..e0d8428806 100644 --- a/pyiceberg/table/update/validate.py +++ b/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}!") diff --git a/tests/table/test_validate.py b/tests/table/test_validate.py index a19983fd66..2ae659fdfe 100644 --- a/tests/table/test_validate.py +++ b/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]]],