-
Notifications
You must be signed in to change notification settings - Fork 287
feat: support incremental scan between 2 snapshots #1470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
crates/iceberg/src/scan/context.rs
Outdated
} | ||
} | ||
} | ||
|
||
struct Ancestors { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be handy for validation in SnapshotProducer
as well, maybe we should move this to a new module util/snapshot.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, what kind of validation are you talking about? It seems currently there's no such place where Ancestors
can be used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snapshot validation is not implemented yet, I have a stale PR for that. My PR also comes with the logic of scanning ancestors, but I like your version much better and would love to reuse what you have here when we restart the snapshot validation work
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
a3f5068
to
d87a9d2
Compare
This refactoring addresses the PR comment suggesting that the Ancestors struct could be reused by SnapshotProducer and should be moved to a dedicated utilities module for better code organization. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
d87a9d2
to
d5fb028
Compare
Which issue does this PR close?
close #1469
What changes are included in this PR?
Added
to_snapshot_id
andfrom_snapshot_id
to the table scan API.Are these changes tested?
unit test