time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.09k stars 273 forks source link

Add ISO8601 duration parsing #571

Closed LukasKalbertodt closed 1 year ago

LukasKalbertodt commented 1 year ago

I saw the really handy functions in the serde module. However, they all just parse OffsetDateTimes, but not durations. At least ISO8601 also defines a syntax for durations (e.g. P1DT4H50M5.1S). Would it be possible to add support for parsing those formats?

jhpratt commented 1 year ago

It's worth pointing out that Duration only has seconds and nanoseconds internally. I do want to add another type (HumanDuration or something) in the future that would support years, months, etc. The advantage is that it could handle DST transitions and leap seconds as well, which Duration fundamentally cannot do. The methods on Duration completely disregard the possibility of a transition, leap second, or other discontinuity. I should make this clearer in the documentation, particularly given my recent work on integrating the time zone database.

So...parsing could be implemented, but it would have to reject all but the simplest of cases, that being ones consisting of only seconds and subsecond values. I suspect that is not what you're looking for.

LukasKalbertodt commented 1 year ago

Thanks for the swift reply. Mh interesting, fair point, I didn't consider that.

I hope this isn't too off-topic, but: would you say that an ISO8601 formatted duration implies that leap seconds and DST are handled? Say I'm receiving a JSON which says "a task has finished". The JSON also carries a duration field, which says how long the task was running. Say I receive that JSON shortly after the time was changed from 2am to 3am due to DST. And the duration field says PT2H. Did the task take 1 hour or 2 hours of processing time? Or is it unclear? I hope my question makes sense. Asked differently: should a "task processing time duration" even be formatted with ISO8601?

jhpratt commented 1 year ago

would you say that an ISO8601 formatted duration implies that leap seconds and DST are handled?

By itself? No. But when the resulting type can be used in arithmetic, there is a reasonable implication that adding "one day" results in the same clock time, even if there was a leap second or DST transition. This is why the standard library has not added methods/constants for minute/hour/day/week. I added them to time, but they still don't handle that. By itself, it's not an issue, but in combination with future plans, it will become one.

tessob commented 1 year ago

Sorry for interruption, but I also have an interest in ISO8601 compatible Duration. The argument that Duration "only has seconds and nanoseconds internally" is clear, however Postgres has nearly the same data representation for the interval type and supports ISO8601 somehow. Just with microsecond precision. Also Postgres has number of arithmetical operations over timestamps & intervals which are pretty similar to Rust traits as Add, Sub and so on.

From my pov Duration looks like a full logical equivalent to Interval.

jhpratt commented 1 year ago

Nearly the same representation isn't sufficient, unfortunately. By adding in "months", postgres is able to losslessly (ignoring leap seconds) represent values containing "months" and "years", which time would be unable to do. The only option on time is to reject these during parsing, which would be quite restrictive — only seconds and subsecond values would be supported, as I do not want the implication that the types handle leap seconds appropriately (this is far more difficult).

tessob commented 1 year ago

@jhpratt From the technical implementation perspective, I represent a philistine view of things. I have never dived deep enough into this topic. When I referred to Postgres that was just and example of possible implementation and I fully understand your points and agree with them.

BTW, probably it makes sense to consider a partial/limited coverage as P1W2DT3H4M5S, where the biggest supported interval/measurement is W and Y and M just can be ignored. This way, as I see it (philistine pov) you can get a valid serialization for any Duration, and deserialization could probably return an error when months or years are provided. For instance Java also uses a subset of this ISO.

Regarding leap seconds, I don't yet understand their implications on a Duration parsing. From my pov (philistine) I need a date and a duration which overlap specific dates when such seconds were added. However here I have too shallow knowledge to argue.

jhpratt commented 1 year ago

At this point in time, I'm going to pass on this due to the serious limitations previously mentioned. If/when a more powerful type is added to time, it will plausibly support this.