Repository navigation
Conversation
…last log entry The snapshot log is not guaranteed to be ordered by timestamp, so iterating it in reverse could return an older snapshot when entries were appended out of order.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Table.snapshot_as_of_timestampwalks the snapshot log backwards and returns the first entry at or before the timestamp, which assumes the log is ordered by timestamp. The spec does not guarantee that, and Java's TableMetadata accepts a log entry up to one minute older than the previous one, so inside that skew window the lookup can return an older snapshot. Java fixed the same assumption inSnapshotUtil.nullableSnapshotIdAsOfTime(apache/iceberg#17360, "finding the entry with the maximum timestamp <= requested timestamp").This picks the entry with the largest timestamp that is at or before (or strictly before, with
inclusive=False) the requested one. On equal timestamps the later entry wins, unlike Java which keeps the first: PyIceberg logs every snapshot added in one transaction with its own timestamp_ms, so two can share a millisecond and the later one is the state that was actually current. Later-wins is also what PyIceberg and Java did before.inspect.metadata_log_entriesuses the same lookup forlatest_snapshot_id.Are these changes tested?
Yes,
test_snapshot_by_timestamp_with_unordered_snapshot_login tests/table/test_init.py (two entries 10 seconds out of order, plus a tie; fails on main, passes here). tests/table/test_init.py and tests/table/test_inspect.py pass.Are there any user-facing changes?
Time travel by timestamp returns the right snapshot when the snapshot log has entries slightly out of timestamp order.