Skip to content

feat(iceberg): Add snapshot utils to scan ancestors#2285

Open
CTTY wants to merge 2 commits intoapache:mainfrom
CTTY:ctty/ancestors
Open

feat(iceberg): Add snapshot utils to scan ancestors#2285
CTTY wants to merge 2 commits intoapache:mainfrom
CTTY:ctty/ancestors

Conversation

@CTTY
Copy link
Collaborator

@CTTY CTTY commented Mar 24, 2026

Which issue does this PR close?

What changes are included in this PR?

  • Add Ancestors to help scan past snapshots
  • Moved existing util to the new utils mod

Are these changes tested?

Yes

@CTTY CTTY marked this pull request as ready for review March 24, 2026 23:28
Copy link
Contributor

@blackmwk blackmwk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @CTTY for this pr!

}

/// Iterate starting from `snapshot` (inclusive) to the root snapshot.
pub fn ancestors_of(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make this part of TableMetadata?

pub fn ancestors_of(
table_metadata: &TableMetadataRef,
snapshot: i64,
) -> Box<dyn Iterator<Item = SnapshotRef> + Send> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a Box here, you can make your iterator return None directly.

}

/// Iterate starting from `snapshot` (inclusive) to `oldest_snapshot_id` (exclusive).
pub fn ancestors_between(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be put into Ancestors struct.


use crate::spec::{SnapshotRef, TableMetadataRef};

struct Ancestors {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we may have more conditions to build iterator, it's more idiomatic to implement IntoIterator for Ancestors, rather than Iterator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add helpers to scan snapshot ancestors

2 participants