mbutrovich commented on code in PR #3260:
URL: https://github.com/apache/iceberg-rust/pull/3260#discussion_r4126173225
##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -207,12 +210,24 @@ impl<'a> TableScanBuilder<'a> {
self
}
- /// Set the snapshot to scan. When not set, it uses current snapshot.
+ /// Set the snapshot to scan. Repeated calls use the last ID.
+ ///
+ /// When neither this nor [`Self::as_of_time`] is set, uses the current
snapshot.
+ /// Combining both selectors causes [`Self::build`] to return an error.
pub fn snapshot_id(mut self, snapshot_id: i64) -> Self {
self.snapshot_id = Some(snapshot_id);
self
}
+ /// Select a snapshot at or before a timestamp in milliseconds since the
Unix epoch.
+ ///
+ /// Uses the table's main history. Repeated calls use the last timestamp.
+ /// Combining this with [`Self::snapshot_id`] causes [`Self::build`] to
return an error.
+ pub fn as_of_time(mut self, timestamp_ms: i64) -> Self {
Review Comment:
Could we add a doctest here, since `as_of_time` is new public API? This one
reads `testdata/example_table_metadata_v2.json` through `StaticTable`, and it
passes with `cargo test -p iceberg --doc` on the head commit.
```suggestion
///
/// ```
/// # use iceberg::TableIdent;
/// # use iceberg::io::FileIO;
/// # use iceberg::table::StaticTable;
/// # #[tokio::main]
/// # async fn main() -> iceberg::Result<()> {
/// # let location = concat!(env!("CARGO_MANIFEST_DIR"),
"/testdata/example_table_metadata_v2.json");
/// # let ident = TableIdent::from_strs(["ns", "t"])?;
/// # let table = StaticTable::from_metadata_file(location, ident,
FileIO::new_with_fs()).await?;
/// // The snapshot log has entries at 1515100955770 and 1555100955770.
/// let scan = table.scan().as_of_time(1_555_100_955_769).build()?;
/// assert_eq!(scan.snapshot().unwrap().snapshot_id(),
3051729675574597004);
/// # Ok(())
/// # }
/// ```
pub fn as_of_time(mut self, timestamp_ms: i64) -> Self {
```
##########
crates/iceberg/src/util/snapshot.rs:
##########
@@ -76,10 +77,38 @@ pub fn ancestors_between(
})
}
+/// Resolve the snapshot ID from the latest main-history entry at or before
+/// `timestamp_ms` (milliseconds since the Unix epoch).
+///
+/// Equal timestamps select the first entry. Returns [`ErrorKind::DataInvalid`]
+/// if no matching history exists. The returned snapshot may have expired, so
+/// [`snapshot_by_id`](crate::spec::TableMetadata::snapshot_by_id) can still
return `None`.
+pub fn snapshot_id_as_of_time(table_metadata: &TableMetadataRef, timestamp_ms:
i64) -> Result<i64> {
Review Comment:
`snapshot_id_as_of_time` is also new public API. Could it get a doctest as
well? This one uses the same fixture, covers a match at an exact log timestamp
and the error before the first entry, and passes with `cargo test -p iceberg
--doc` on the head commit.
```suggestion
///
/// ```
/// # use iceberg::TableIdent;
/// # use iceberg::io::FileIO;
/// # use iceberg::table::StaticTable;
/// use iceberg::util::snapshot::snapshot_id_as_of_time;
/// # #[tokio::main]
/// # async fn main() -> iceberg::Result<()> {
/// # let location = concat!(env!("CARGO_MANIFEST_DIR"),
"/testdata/example_table_metadata_v2.json");
/// # let ident = TableIdent::from_strs(["ns", "t"])?;
/// # let table = StaticTable::from_metadata_file(location, ident,
FileIO::new_with_fs()).await?;
/// let metadata = table.metadata();
/// // The snapshot log has entries at 1515100955770 and 1555100955770.
/// let snapshot_id = snapshot_id_as_of_time(&metadata, 1_555_100_955_770)?;
/// assert_eq!(snapshot_id, 3055729675574597004);
/// assert!(snapshot_id_as_of_time(&metadata, 1_515_100_955_769).is_err());
/// # Ok(())
/// # }
/// ```
pub fn snapshot_id_as_of_time(table_metadata: &TableMetadataRef,
timestamp_ms: i64) -> Result<i64> {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
