-
Notifications
You must be signed in to change notification settings - Fork 596
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
feat(types): support ns timestamp #19827
Open
xxhZs
wants to merge
9
commits into
main
Choose a base branch
from
xxh/support-timestamp-ns-1
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+226
−36
Open
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
efcb97b
support
xxhZs 4aad830
fix comm
xxhZs 9bf25a9
Merge branch 'main' into xxh/support-timestamp-ns-1
wcy-fdu a6acfc2
add ci
xxhZs 026e08a
fix comm
xxhZs f1cb8ab
fix fmt
xxhZs d09d463
add doc + fix ci
xxhZs 2c87c76
fix ci
xxhZs d975bf0
fix ci
xxhZs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,29 +168,18 @@ impl FromStr for Timestamp { | |
type Err = InvalidParamsError; | ||
|
||
fn from_str(s: &str) -> Result<Self> { | ||
if let Ok(res) = speedate::DateTime::parse_str_rfc3339(s) { | ||
if res.time.tz_offset.is_some() { | ||
return Err(ErrorKind::ParseTimestamp.into()); | ||
} | ||
Ok(Date::from_ymd_uncheck( | ||
res.date.year as i32, | ||
res.date.month as u32, | ||
res.date.day as u32, | ||
) | ||
.and_hms_micro_uncheck( | ||
res.time.hour as u32, | ||
res.time.minute as u32, | ||
res.time.second as u32, | ||
res.time.microsecond, | ||
)) | ||
} else { | ||
let res = | ||
speedate::Date::parse_str_rfc3339(s).map_err(|_| ErrorKind::ParseTimestamp)?; | ||
Ok( | ||
Date::from_ymd_uncheck(res.year as i32, res.month as u32, res.day as u32) | ||
.and_hms_micro_uncheck(0, 0, 0, 0), | ||
) | ||
} | ||
let dt = s | ||
.parse::<jiff::civil::DateTime>() | ||
.map_err(|_| ErrorKind::ParseTimestamp)?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error message of |
||
Ok( | ||
Date::from_ymd_uncheck(dt.year() as i32, dt.month() as u32, dt.day() as u32) | ||
.and_hms_nano_uncheck( | ||
dt.hour() as u32, | ||
dt.minute() as u32, | ||
dt.second() as u32, | ||
dt.subsec_nanosecond() as u32, | ||
), | ||
) | ||
} | ||
} | ||
|
||
|
@@ -422,6 +411,13 @@ impl Date { | |
.and_time(Time::from_hms_micro_uncheck(hour, min, sec, micro).0), | ||
) | ||
} | ||
|
||
pub fn and_hms_nano_uncheck(self, hour: u32, min: u32, sec: u32, nano: u32) -> Timestamp { | ||
Timestamp::new( | ||
self.0 | ||
.and_time(Time::from_hms_nano_uncheck(hour, min, sec, nano).0), | ||
) | ||
} | ||
} | ||
|
||
impl Time { | ||
|
@@ -495,18 +491,24 @@ impl Timestamp { | |
} | ||
|
||
pub fn from_protobuf(cur: &mut Cursor<&[u8]>) -> ArrayResult<Timestamp> { | ||
let micros = cur | ||
let secs = cur | ||
xxhZs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.read_i64::<BigEndian>() | ||
.context("failed to read i64 from Timestamp buffer")?; | ||
.context("failed to read i64 from Time buffer")?; | ||
let nsecs = cur | ||
.read_u32::<BigEndian>() | ||
.context("failed to read u32 from Time buffer")?; | ||
|
||
Ok(Timestamp::with_micros(micros)?) | ||
Ok(Timestamp::with_secs_nsecs(secs, nsecs)?) | ||
} | ||
|
||
/// Although `Timestamp` takes 12 bytes, we drop 4 bytes in protobuf encoding. | ||
pub fn to_protobuf<T: Write>(self, output: &mut T) -> ArrayResult<usize> { | ||
output | ||
.write(&(self.0.and_utc().timestamp_micros()).to_be_bytes()) | ||
.map_err(Into::into) | ||
let timestamp_size = output | ||
.write(&(self.0.and_utc().timestamp()).to_be_bytes()) | ||
.map_err(Into::<ArrayError>::into)?; | ||
let timestamp_subsec_nanos_size = output | ||
.write(&(self.0.and_utc().timestamp_subsec_nanos()).to_be_bytes()) | ||
.map_err(Into::<ArrayError>::into)?; | ||
Ok(timestamp_subsec_nanos_size + timestamp_size) | ||
xxhZs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
pub fn get_timestamp_nanos(&self) -> i64 { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why do we need to introduce yet another dependency in addition to
chrono
andspeedate
?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.
Because chrono can't be parsed, and speedate is only parsed at the us level