bitwalker / timex

A complete date/time library for Elixir projects.
https://hexdocs.pm/timex
MIT License
1.76k stars 379 forks source link

Adding a month to a date gives wrong result #430

Closed davidmoreno closed 6 years ago

davidmoreno commented 6 years ago

Steps to reproduce

month = Timex.Duration.parse!("P1M")
Timex.add( Timex.now(), month )

Description of issue

Expected one month later, same day of month:

"2018-05-24 15:08:07.225464Z" + "P1M" == "2018-06-24 15:08:07.225464Z"

But I get:

"2018-06-23 15:08:07.225464Z"

I'm not sure if this is a bug or a feature request, as the documentation clearly states this is a wrapper over Erlang timestamps. Anyway the expected result is not as expected.

Because of this erlang timestamp is in seconds, it is not a valid structure to store durations as month or year, and some may argue even days and weeks (leap seconds).

davidmoreno commented 6 years ago

As a workaround for my project I implemented an ISO8601 duration parser and adder:

https://github.com/serverboards/exosql/blob/master/lib/datetime.ex#L121

Stores each part as proper duration, and depending on content calls the Timex.shift function.

It may be optimized to just store the duration in the format that Timex.shift wants, using keyword list instead of a struct.

sascha-wolf commented 6 years ago

We've encountered this issue too.

iex> {:ok, date_time, _} = DateTime.from_iso8601("2018-07-31T08:37:35.192654Z")
{:ok, #DateTime<2018-07-31 08:37:35.192654Z>, 0}
iex> Timex.shift(date_time, months: -1)
#DateTime<2018-07-01 08:37:35.192654Z>

As you can see, shifting backwards by one month on the 31. of July results in the 1. of July and not the 30. of Juni, as I would have expected.

sascha-wolf commented 6 years ago

Interestingly enough shifting a Date or a NaiveDateTime works as expected. Only DateTime seems to result in troubles.

ckhrysze commented 6 years ago

@davidmoreno Thank you for reporting this issue! Although non-ideal, his is working as intended, given the vagueness of what a month is. The ISO standard even has a nod toward this issue, and notes that for some applications '30 days' makes sense for a month, which is what Timex does. However, shift behaves differently, and was fixed with 3.3 to work your scenario:

iex(1)> {:ok, date_time, _} = DateTime.from_iso8601("2018-05-24 15:08:07.225464Z")
{:ok, #DateTime<2018-05-24 15:08:07.225464Z>, 0}
iex(2)> Timex.shift(date_time, months: 1)
#DateTime<2018-06-24 15:08:07.225464Z>

as well as @Zeeker 's 3.2.2

iex(1)> {:ok, date_time, _} = DateTime.from_iso8601("2018-07-31T08:37:35.192654Z")
{:ok, #DateTime<2018-07-31 08:37:35.192654Z>, 0}
iex(2)> Timex.shift(date_time, months: -1)
#DateTime<2018-07-01 08:37:35.192654Z>

3.3.0

iex(1)> {:ok, date_time, _} = DateTime.from_iso8601("2018-07-31T08:37:35.192654Z")
{:ok, #DateTime<2018-07-31 08:37:35.192654Z>, 0}
iex(2)> Timex.shift(date_time, months: -1)
#DateTime<2018-06-30 08:37:35.192654Z>