Skip to content
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

Suggestion: time::Time precision and explicit Timestamp type #729

Open
exldna opened this issue Feb 15, 2025 · 1 comment
Open

Suggestion: time::Time precision and explicit Timestamp type #729

exldna opened this issue Feb 15, 2025 · 1 comment
Labels
A-core Area: anything not otherwise covered C-blocked Category: blocked by another (possibly upstream) issue C-feature-request Category: a new feature (not already implemented)

Comments

@exldna
Copy link

exldna commented Feb 15, 2025

Hi, I'm a beginner in Rust and I don't know the whole story of this crate, it's my first issue in this repository.

My suggestion consists of two parts:

  • Time precision
  • Timestamp

I think that it is important to talk about both parts in one issue, because of the interaction between them.

about the time::Time precision

It is often needs to relax time precision requirements. Suppose we need to design the train tickets selling system. So we need the ticket representation in our program. Every ticket has a departure time and an arriving time and we want show it in our representation. Usually, trains schedule builds with minute precision (without seconds or nanoseconds), so we want to store the minutes and hours of departure and arriving. And, as trains may travel long distances, we want to store the date and time with utc offset. Summarizing, time::OffsetDateTime is most suitable type for us. And here we got into trouble. If we want to be able to compare departure or arriving times, we must keep invariant that nanoseconds (and seconds) will always be zero. And the library will not help us. It is all on us. Also we have to store at least 5 extra unused bytes for each time instance in our program.

I suggest to provide different Time types for each one precision quality and add suitable cross-operations between them. I would like to discuss the exact design, but for further reading suppose we have type Time with generic parameter precision.

about the Timestamp

Current unix timestamp support is quite not ergonomic. All we have are just couple of methods on time::OffesetDateTime to convert from/into unix timestamp that actually i64, but there is no way to keep the timestamp invariant. I suggest to add explicit Timestamp type that is transparently i64 for backwards compatibility.

Here are my arguments, explaining why this type seems reasonable:

  1. Converting between time::OffesetDateTime and Timestamp do not require any explicit checks for users, because the type system guarantees that if timestamp value exists, it is correct. And any valid timestamp may be safely converted into time::OffsetDateTime. This may seem like just transferring responsibility to another type, but it is not. Once created and checked, the 'Timestamp' value can be used without further checks. (unless we need to change it)
  2. Timestamp is defined as numbers of seconds, and if we want to keep nanoseconds, we must store it explicitly. That's why I want to suggest to define another type TimestampNanos to handle this case. So, the relations between the timestamp values and the related nanoseconds will not get messed up. But the Timestamp type is still important, because we cannot safely create the Time<Precision::Seconds> (or whatever) from TimestampNanos.

However, there is an easier way. We may just add the unsafe from_unix_timestamp_unchecked method to time::OffesetDateTime. And users will have the ability to write their own wrappers.

Also I picked simplified Timestamp version, but it is also possible to design Timestamp with other kinds of precision, not just nanoseconds. Maybe we can reuse sub second precision kinds from Time in this case?

Bringing all together

  • Discuss Time precision design. My thoughts are that we should define the new time type to keep time::Time for compatibility. I would like to see something like Precision enum, but there are a lot of nuances.
  • Add Timestamp and TimestampNanos types.
  • Add conversions between Time with precision seconds and Timestamp, Time with precision nanoseconds and TimestampNanos.
@jhpratt jhpratt added C-feature-request Category: a new feature (not already implemented) A-core Area: anything not otherwise covered labels Feb 17, 2025
@jhpratt
Copy link
Member

jhpratt commented Feb 17, 2025

There is absolutely no way you would have known this — I never wrote anything publicly or pushed relevant code — but I have tried to implement limited precision for Time before. This is absolutely something I want to do. Unfortunately, doing so ergonomically is quite difficult and requires a handful of nightly features to do it in the manner I would like. Notably, I do not want to create a separate type, but retrofit it onto the existing set of types.

I'm sure there are more, but a quick look shows at least these features that are required or most likely required before this could happen:

A dedicated type for timestamps is doable, but would involve similar challenges. It would also be more difficult to add in a backwards-compatible way (though possibly easier with const traits) as the existing methods return primitive types.

We may just add the unsafe from_unix_timestamp_unchecked method to time::OffesetDateTime. And users will have the ability to write their own wrappers.

This is doable, but it would be no different than .unwrap_unchecked() on the result of from_unix_timestamp.

@jhpratt jhpratt added the C-blocked Category: blocked by another (possibly upstream) issue label Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: anything not otherwise covered C-blocked Category: blocked by another (possibly upstream) issue C-feature-request Category: a new feature (not already implemented)
Projects
None yet
Development

No branches or pull requests

2 participants