chinmay-bhat commented on code in PR #748:
URL: https://github.com/apache/iceberg-python/pull/748#discussion_r1616712540
##########
pyiceberg/table/__init__.py:
##########
@@ -1290,6 +1291,19 @@ def snapshot_by_name(self, name: str) ->
Optional[Snapshot]:
return self.snapshot_by_id(ref.snapshot_id)
return None
+ def latest_snapshot_before_timestamp(self, timestamp_ms: int) ->
Optional[Snapshot]:
+ """Get the snapshot right before the given timestamp, or None if there
is no matching snapshot."""
+ result, prev_timestamp = None, 0
+ if self.metadata.current_snapshot_id is not None:
+ for snapshot in self.current_ancestors():
+ if snapshot and prev_timestamp < snapshot.timestamp_ms <
timestamp_ms:
Review Comment:
> I think we could also include the given timestamp, making the method
return the latest snapshot up until the given timestamp.
That was my initial approach, however, I saw the [java
implementation](https://github.com/apache/iceberg/blob/795fea9446723cf9c2dbc3cac07883cace8997c9/core/src/main/java/org/apache/iceberg/SetSnapshotOperation.java#L146)
for rollback which is not inclusive of the given timestamp. This
[implementation](https://github.com/apache/iceberg/blob/795fea9446723cf9c2dbc3cac07883cace8997c9/core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java#L350),
which is what you describe, exists but is not used for rollback. So I thought
there must be a design decision I am unaware of to keep it not inclusive.
WDYT - should I still change it to be inclusive of the timestamp?
> Also, how about implementing this by iterating over table.history()? The
snapshot_log field in metadata contains a list of snapshot_id + timestamp pair
so we do not need to re-generate the ancestors for current snapshot.
Ok!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
