feat(transaction): Support snapshot validation#1353
feat(transaction): Support snapshot validation#1353CTTY wants to merge 7 commits intoapache:mainfrom
Conversation
|
I'll tkae a look later today! |
| base: &Table, | ||
| to_snapshot: &SnapshotRef, | ||
| from_snapshot: Option<&SnapshotRef>, |
There was a problem hiding this comment.
I think should swap to and from here, @sungwy cleaned up my original implementation via https://github.com/apache/iceberg-python/pull/1959/files
| fn ancestors_between( | ||
| to_snapshot: &SnapshotRef, | ||
| from_snapshot: Option<&SnapshotRef>, |
There was a problem hiding this comment.
Should this live somewhere else? idrk but it does in the python impl
Also the to and from are swapped
Also might be more readable to refactor out the ancestors_of logic like we have in python
There was a problem hiding this comment.
Yes, I believe the validation should be a part of snapshot producing that happens when committing certain transaction actions.
This work has been paused and we are trying to make iceberg-rust able to write data first: #1382
I'll certain check out pyiceberg when resuming this!
|
This PR is stale for now, I've included a new version of snapshot validation in my big draft #1606 . I'll port the changes over once it's ready |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
3 participants
Which issue does this PR close?
What changes are included in this PR?
SnapshotValidatorto hold these logicAre these changes tested?