fix to/from logic and allow optional to_snapshot arg in validation_history

Author: jayceslesar

pyiceberg/table/snapshots.py

@@ -432,14 +432,13 @@ def ancestors_of(current_snapshot: Optional[Snapshot], table_metadata: TableMeta
 
 
 def ancestors_between(
-    current_snapshot: Snapshot, oldest_snapshot: Optional[Snapshot], table_metadata: TableMetadata
+    to_snapshot: Snapshot, from_snapshot: Optional[Snapshot], table_metadata: TableMetadata
 ) -> Iterable[Snapshot]:
     """Get the ancestors of and including the given snapshot between the latest and oldest snapshot."""
-    if oldest_snapshot is not None:
-        for snapshot in ancestors_of(current_snapshot, table_metadata):
-            if snapshot.snapshot_id == oldest_snapshot.snapshot_id:
+    if from_snapshot is not None:
+        for snapshot in ancestors_of(to_snapshot, table_metadata):
+            if snapshot == from_snapshot:
                 break
             yield snapshot
     else:
-        for snapshot in ancestors_of(current_snapshot, table_metadata):
-            yield snapshot
+        yield from ancestors_of(to_snapshot, table_metadata)

pyiceberg/table/update/validate.py

@@ -14,6 +14,8 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+from typing import Optional
+
 from pyiceberg.manifest import ManifestContent, ManifestFile
 from pyiceberg.table import Table
 from pyiceberg.table.snapshots import Operation, Snapshot, ancestors_between
@@ -25,17 +27,17 @@ class ValidationException(Exception):
 
 def validation_history(
     table: Table,
-    starting_snapshot: Snapshot,
-    parent_snapshot: Snapshot,
+    from_snapshot: Snapshot,
+    to_snapshot: Optional[Snapshot],
     matching_operations: set[Operation],
     manifest_content_filter: ManifestContent,
 ) -> tuple[list[ManifestFile], set[Snapshot]]:
     """Return newly added manifests and snapshot IDs between the starting snapshot and parent snapshot.
 
     Args:
         table: Table to get the history from
-        starting_snapshot: Starting snapshot
-        parent_snapshot: Parent snapshot to get the history from
+        from_snapshot: Parent snapshot to get the history from
+        to_snapshot: Starting snapshot
         matching_operations: Operations to match on
         manifest_content_filter: Manifest content type to filter
 
@@ -49,7 +51,7 @@ def validation_history(
     snapshots: set[Snapshot] = set()
 
     last_snapshot = None
-    for snapshot in ancestors_between(starting_snapshot, parent_snapshot, table.metadata):
+    for snapshot in ancestors_between(from_snapshot, to_snapshot, table.metadata):
         last_snapshot = snapshot
         summary = snapshot.summary
         if summary is None:
@@ -64,7 +66,7 @@ def validation_history(
                 ]
             )
 
-    if last_snapshot is None or last_snapshot.snapshot_id == starting_snapshot.snapshot_id:
+    if last_snapshot is None or last_snapshot.snapshot_id == from_snapshot.snapshot_id:
         raise ValidationException("No matching snapshot found.")
 
     return manifests_files, snapshots

tests/table/test_snapshots.py

@@ -15,6 +15,8 @@
 # specific language governing permissions and limitations
 # under the License.
 # pylint:disable=redefined-outer-name,eval-used
+from typing import cast
+
 import pytest
 
 from pyiceberg.manifest import DataFile, DataFileContent, ManifestContent, ManifestFile
@@ -392,11 +394,12 @@ def test_ancestors_of_recursive_error(table_v2_with_extensive_snapshots: Table)
 
 def test_ancestors_between(table_v2_with_extensive_snapshots: Table) -> None:
     oldest_snapshot = table_v2_with_extensive_snapshots.snapshots()[0]
+    current_snapshot = cast(Snapshot, table_v2_with_extensive_snapshots.current_snapshot())
     assert (
         len(
             list(
                 ancestors_between(
-                    table_v2_with_extensive_snapshots.current_snapshot(),
+                    current_snapshot,
                     oldest_snapshot,
                     table_v2_with_extensive_snapshots.metadata,
                 )