Skip to content

Commit d5fb028

Browse files
xxchanclaude
andcommitted
refactor: move Ancestors snapshot utilities to util/snapshot.rs
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]>
1 parent 550d990 commit d5fb028

File tree

4 files changed

+94
-54
lines changed

4 files changed

+94
-54
lines changed

crates/iceberg/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,5 @@ pub mod writer;
8787

8888
mod delete_vector;
8989
pub mod puffin;
90+
/// Utility functions and modules.
91+
pub mod util;

crates/iceberg/src/scan/context.rs

Lines changed: 1 addition & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use crate::spec::{
3333
DataContentType, ManifestContentType, ManifestEntryRef, ManifestFile, ManifestList,
3434
ManifestStatus, Operation, SchemaRef, SnapshotRef, TableMetadataRef,
3535
};
36+
use crate::util::snapshot::ancestors_between;
3637
use crate::{Error, ErrorKind, Result};
3738

3839
type ManifestEntryFilterFn = dyn Fn(&ManifestEntryRef) -> bool + Send + Sync;
@@ -349,57 +350,3 @@ impl PlanContext {
349350
}
350351
}
351352
}
352-
353-
struct Ancestors {
354-
next: Option<SnapshotRef>,
355-
get_snapshot: Box<dyn Fn(i64) -> Option<SnapshotRef> + Send>,
356-
}
357-
358-
impl Iterator for Ancestors {
359-
type Item = SnapshotRef;
360-
361-
fn next(&mut self) -> Option<Self::Item> {
362-
let snapshot = self.next.take()?;
363-
let result = snapshot.clone();
364-
self.next = snapshot
365-
.parent_snapshot_id()
366-
.and_then(|id| (self.get_snapshot)(id));
367-
Some(result)
368-
}
369-
}
370-
371-
/// Iterate starting from `snapshot` (inclusive) to the root snapshot.
372-
fn ancestors_of(
373-
table_metadata: &TableMetadataRef,
374-
snapshot: i64,
375-
) -> Box<dyn Iterator<Item = SnapshotRef> + Send> {
376-
if let Some(snapshot) = table_metadata.snapshot_by_id(snapshot) {
377-
let table_metadata = table_metadata.clone();
378-
Box::new(Ancestors {
379-
next: Some(snapshot.clone()),
380-
get_snapshot: Box::new(move |id| table_metadata.snapshot_by_id(id).cloned()),
381-
})
382-
} else {
383-
Box::new(std::iter::empty())
384-
}
385-
}
386-
387-
/// Iterate starting from `snapshot` (inclusive) to `oldest_snapshot_id` (exclusive).
388-
fn ancestors_between(
389-
table_metadata: &TableMetadataRef,
390-
latest_snapshot_id: i64,
391-
oldest_snapshot_id: Option<i64>,
392-
) -> Box<dyn Iterator<Item = SnapshotRef> + Send> {
393-
let Some(oldest_snapshot_id) = oldest_snapshot_id else {
394-
return Box::new(ancestors_of(table_metadata, latest_snapshot_id));
395-
};
396-
397-
if latest_snapshot_id == oldest_snapshot_id {
398-
return Box::new(std::iter::empty());
399-
}
400-
401-
Box::new(
402-
ancestors_of(table_metadata, latest_snapshot_id)
403-
.take_while(move |snapshot| snapshot.snapshot_id() != oldest_snapshot_id),
404-
)
405-
}

crates/iceberg/src/util/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
/// Utilities for working with snapshots.
19+
pub mod snapshot;

crates/iceberg/src/util/snapshot.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
use crate::spec::{SnapshotRef, TableMetadataRef};
19+
20+
struct Ancestors {
21+
next: Option<SnapshotRef>,
22+
get_snapshot: Box<dyn Fn(i64) -> Option<SnapshotRef> + Send>,
23+
}
24+
25+
impl Iterator for Ancestors {
26+
type Item = SnapshotRef;
27+
28+
fn next(&mut self) -> Option<Self::Item> {
29+
let snapshot = self.next.take()?;
30+
let result = snapshot.clone();
31+
self.next = snapshot
32+
.parent_snapshot_id()
33+
.and_then(|id| (self.get_snapshot)(id));
34+
Some(result)
35+
}
36+
}
37+
38+
/// Iterate starting from `snapshot` (inclusive) to the root snapshot.
39+
pub fn ancestors_of(
40+
table_metadata: &TableMetadataRef,
41+
snapshot: i64,
42+
) -> Box<dyn Iterator<Item = SnapshotRef> + Send> {
43+
if let Some(snapshot) = table_metadata.snapshot_by_id(snapshot) {
44+
let table_metadata = table_metadata.clone();
45+
Box::new(Ancestors {
46+
next: Some(snapshot.clone()),
47+
get_snapshot: Box::new(move |id| table_metadata.snapshot_by_id(id).cloned()),
48+
})
49+
} else {
50+
Box::new(std::iter::empty())
51+
}
52+
}
53+
54+
/// Iterate starting from `snapshot` (inclusive) to `oldest_snapshot_id` (exclusive).
55+
pub fn ancestors_between(
56+
table_metadata: &TableMetadataRef,
57+
latest_snapshot_id: i64,
58+
oldest_snapshot_id: Option<i64>,
59+
) -> Box<dyn Iterator<Item = SnapshotRef> + Send> {
60+
let Some(oldest_snapshot_id) = oldest_snapshot_id else {
61+
return Box::new(ancestors_of(table_metadata, latest_snapshot_id));
62+
};
63+
64+
if latest_snapshot_id == oldest_snapshot_id {
65+
return Box::new(std::iter::empty());
66+
}
67+
68+
Box::new(
69+
ancestors_of(table_metadata, latest_snapshot_id)
70+
.take_while(move |snapshot| snapshot.snapshot_id() != oldest_snapshot_id),
71+
)
72+
}

0 commit comments

Comments
 (0)