Fix ancestors_of exception for tables with >=2000 snapshots

[ndrluis]

When I was running some tests in production, I discovered this error.

[sungwy]

Hi @ndrluis - thank you for raising this. This looks like an issue we'd definitely want to fix.

Just to help with reviewing this PR, and for future records, would you be able to share the stack trace of the exception you are running into in this edge case?

[ndrluis]

Hi @syun64, this is the stacktrace

Traceback (most recent call last):
  File "/Users/magus/Workspace/oss/iceberg-python/test_view.py", line 18, in <module>
    print(list(ancestors_of(t.current_snapshot(), t.metadata)))
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 428, in ancestors_of
    yield from ancestors_of(parent, table_metadata)
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 428, in ancestors_of
    yield from ancestors_of(parent, table_metadata)
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 428, in ancestors_of
    yield from ancestors_of(parent, table_metadata)
  [Previous line repeated 992 more times]
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 427, in ancestors_of
    if parent := table_metadata.snapshot_by_id(current_snapshot.parent_snapshot_id):
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/metadata.py", line 231, in snapshot_by_id
    return next((snapshot for snapshot in self.snapshots if snapshot.snapshot_id == snapshot_id), None)
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/metadata.py", line 231, in <genexpr>
    return next((snapshot for snapshot in self.snapshots if snapshot.snapshot_id == snapshot_id), None)
RecursionError: maximum recursion depth exceeded in comparison

[sungwy]

Hi @syun64, this is the stacktrace

Traceback (most recent call last):
  File "/Users/magus/Workspace/oss/iceberg-python/test_view.py", line 18, in <module>
    print(list(ancestors_of(t.current_snapshot(), t.metadata)))
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 428, in ancestors_of
    yield from ancestors_of(parent, table_metadata)
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 428, in ancestors_of
    yield from ancestors_of(parent, table_metadata)
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 428, in ancestors_of
    yield from ancestors_of(parent, table_metadata)
  [Previous line repeated 992 more times]
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/snapshots.py", line 427, in ancestors_of
    if parent := table_metadata.snapshot_by_id(current_snapshot.parent_snapshot_id):
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/metadata.py", line 231, in snapshot_by_id
    return next((snapshot for snapshot in self.snapshots if snapshot.snapshot_id == snapshot_id), None)
  File "/Users/magus/Workspace/oss/iceberg-python/pyiceberg/table/metadata.py", line 231, in <genexpr>
    return next((snapshot for snapshot in self.snapshots if snapshot.snapshot_id == snapshot_id), None)
RecursionError: maximum recursion depth exceeded in comparison

Hey @ndrluis thanks for sharing the stacktrace. If the issue is with the recursion depth, could we try writing this in a simple way that doesn't require recursion?

i.e.

def ancestors_of(current_snapshot: Optional[Snapshot], table_metadata: TableMetadata) -> Iterable[Snapshot]:
    snapshot = current_snapshot
    while snapshot is not None:
        yield snapshot
        snapshot = table_metadata.snapshot_by_id(current_snapshot.parent_snapshot_id)

[ndrluis]

@syun64 Thank you for the review. I made the changes as you asked and also made a slight change due to a mypy error regarding the snapshot_by_id type Optional[int].

[sungwy]

LGTM

[Fokko]

@ndrluis Thanks, looking good 👍 @syun64 appreciate the review 🙌