I was wondering why the feature
Int#[seconds..day,month] has been cut in two different TimeSpan?
In crystal, it’s currently impossible to write:
x = 1.month + 1.day
The performance impact of having only unified TimeSpan is very limited (store 8 more bytes per TimeSpan). And in case of “logic” issue, I think we should just add the months first in any case, before adding the days:
# now -> "29-01-2018"
time_span = 1.day + 1.month
now + time_span # -> 01-03-2018 (We add first the month, leading to 28th february then add the day.)
INTERVAL is stored as:
And here is the result:
SELECT DATE('2018-01-28') + INTERVAL '1 day 1 month';
-- 2018-03-01 00:00:00
SELECT DATE('2018-01-28') + INTERVAL '1 month 1 day';
-- 2018-03-01 00:00:00
in ActiveSupport however, things start to get really confusing:
irb(main):012:0> DateTime.parse("2018-01-28") + (1.day + 1.month)
=> Wed, 28 Feb 2018 00:00:00 +0000
irb(main):011:0> DateTime.parse("2018-01-28") + (1.month + 1.day)
=> Thu, 01 Mar 2018 00:00:00 +0000
puts Time.new(2018,1,28) + 1.month + 1.day
# > 2018-03-01 00:00:00 +07:00
puts Time.new(2018,1,28) + 1.day + 1.month
# > 2018-02-28 00:00:00 +07:00
Like active record
puts Time.new(2018,1,28) + (1.day + 1.month)
# Error in XXXX: no overload matches 'Time::Span#+' with type Time::MonthSpan
# Overloads are:
# - Time::Span#+(other : self)
# - Time::Span#+()
I honestly 100% prefer the PostgreSQL version of Interval, which sounds less error prone imho.
SELECT DATE('2018-01-28') + INTERVAL '1 day' + INTERVAL '1 month';
-- 2018-02-28 00:00:00
1.month has no meaning on its own, it’s dependent on which month we’re talking about. Whereas 1.day is always the same.
I agree, we should unify the spans. Please open an issue in the repository. Thank you!
A month is always a month. There are no problems with adding ( or subtracting) a month from a time instance.
Ambiguity only exist when converting the time span of “1 month” to a smaller duration unit like days.
In a hilarious coincidence, I just opened PR #7454 to address this because Neo4j’s
Duration type (one of its new temporal types) couldn’t map to anything in the Neo4j driver.
LocalDateTime is possible due because Crystal has
Time::Location.local, which is amazing, so I wanted to stick with the core library as much as possible.
Awesome coincidence , indeed in my case it’s for mapping of Interval object in Postgres/Clear ORM. I’ll review the PR now then