Time::Span and Time::MonthSpan

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.)
1 Like

In Postgres, INTERVAL is stored as:

  @months: Int32
  @days: Int32
  @microseconds: Int64

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

Quite coherent :slight_smile:

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


In crystal:

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 :thinking:

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.

EDIT: Insolvable?

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!

1 Like

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.

Neo4j’s 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 :stuck_out_tongue:, indeed in my case it’s for mapping of Interval object in Postgres/Clear ORM. I’ll review the PR now then :slight_smile: