Xuanwo commented on code in PR #331:
URL: https://github.com/apache/iceberg-rust/pull/331#discussion_r1693949016
##########
crates/iceberg/src/error.rs:
##########
@@ -333,6 +335,28 @@ define_from_err!(
define_from_err!(std::io::Error, ErrorKind::Unexpected, "IO Operation failed");
+/// Converts a timestamp in milliseconds to `DateTime<Utc>`, handling errors.
+///
+/// # Arguments
+///
+/// * `timestamp_ms` - The timestamp in milliseconds to convert.
+///
+/// # Returns
+///
+/// This function returns a `Result<DateTime<Utc>, Error>` which is `Ok` with
the `DateTime<Utc>` if the conversion is successful,
+/// or an `Err` with an appropriate error if the timestamp is ambiguous or
invalid.
+pub(crate) fn timestamp_ms_to_utc(timestamp_ms: i64) -> Result<DateTime<Utc>> {
+ match Utc.timestamp_millis_opt(timestamp_ms) {
+ chrono::LocalResult::Single(t) => Ok(t),
+ chrono::LocalResult::Ambiguous(_, _) => Err(Error::new(
Review Comment:
> I am not quite sure what do you mean with detailed info in error - we
already have the context "timestamp value". Would you add a more verbose
message?
I want to carry the info returned in `LocalResult::Ambiguous(_, _)`. But I'm
guessing they are not useful?
##########
crates/iceberg/src/error.rs:
##########
@@ -333,6 +335,28 @@ define_from_err!(
define_from_err!(std::io::Error, ErrorKind::Unexpected, "IO Operation failed");
+/// Converts a timestamp in milliseconds to `DateTime<Utc>`, handling errors.
+///
+/// # Arguments
+///
+/// * `timestamp_ms` - The timestamp in milliseconds to convert.
+///
+/// # Returns
+///
+/// This function returns a `Result<DateTime<Utc>, Error>` which is `Ok` with
the `DateTime<Utc>` if the conversion is successful,
+/// or an `Err` with an appropriate error if the timestamp is ambiguous or
invalid.
+pub(crate) fn timestamp_ms_to_utc(timestamp_ms: i64) -> Result<DateTime<Utc>> {
+ match Utc.timestamp_millis_opt(timestamp_ms) {
+ chrono::LocalResult::Single(t) => Ok(t),
+ chrono::LocalResult::Ambiguous(_, _) => Err(Error::new(
Review Comment:
> I am not quite sure what do you mean with detailed info in error - we
already have the context "timestamp value". Would you add a more verbose
message?
I want to carry the info returned in `LocalResult::Ambiguous(_, _)`. But I'm
guessing they are not very useful?
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
