ARROW-13977: [Format] clarify leap seconds for interval type

[houqp]

Like other time units, we don't account for leap seconds for interval types. For MONTH_DAY_NANO, we do account for leap days.

[github-actions]

https://issues.apache.org/jira/browse/ARROW-13977

[github-actions]

⚠️ Ticket has no components in JIRA, make sure you assign one.

[github-actions]

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

[houqp]

For the context, this came out of the exercise to define partial order for Interval Types in jorgecarleitao/arrow2#398.

[jorgecarleitao]

If we do not account for leap seconds, isn't this type equivalent to Duration[ms]?

I.e. Given MS_IN_DAY = 60*60*24*1000, if Interval(1d,MS_IN_DAY) == Interval(2d,0), then Interval(a,b) == Duration[ms](a*MS_IN_DAY + b) which makes them semantically equivalent (and thus redundant?). This is how I would conclude that Interval(1d,MS_IN_DAY) != Interval(2d,0), i.e. to capture different semantics.

OTOH, many systems do not handle leap seconds and in 2012 there were no plans to support leap seconds in Postgres.

So, my conclusion atm is that the decision tree is

care about leap	Type
days	Interval MonthDaysNs
seconds	Interval DaysMs
days and seconds	Extension
None	Duration

If we ignore leap seconds in DaysMs, the table would become

care about leap	Type
days	Interval MonthDaysNs
seconds	Extension
days and seconds	Extension
None	Duration

and we may deprecate DaysMs without loss of generality?

---

Since DataFusion follows postgres, I shouldn't we stick to MONTHS_DAYS_NS so that we capture its behavior of storing months, days and (nano)seconds separately,

postgres=# SELECT  interval '11 months + 28 days + 86399 seconds' + interval '1 month + 1 day + 1 second';
        ?column?         
-------------------------
 1 year 29 days 24:00:00
(1 row)

[houqp]

@jorgecarleitao even if we don't account for leap seconds, Interval(1d,MS_IN_DAY) is still not equal to Interval(2d,0) due to daylight saving adjustments. Here is the quote from the comment:

// A "calendar" interval which models types that don't necessarily
// have a precise duration without the context of a base timestamp (e.g.
// days can differ in length during day light savings time transitions).

Since DataFusion follows postgres, I shouldn't we stick to MONTHS_DAYS_NS so that we capture its behavior of storing months, days and (nano)seconds separately,

Are you suggesting that we should use MONTHS_DAYS_NS everywhere in datafusion and drop day_ms entirely?

[jorgecarleitao]

You are right 👍

I understood that comment to differentiate it from Duration, that denotes a physical time interval. Specifically, my understanding is that Duration[s](1) equals 1 second as per its definition of atomic clocks and stuff.

In opposition to an Interval, that corresponds time units that are constructs associated with a calendar (historically associated with the Earth and moon movements), such as "Month", "day", etc.

The main difference (in my understanding) is that add_duration(now,duration(MS_IN_DAY)) - now = duration(MS_IN_DAY) and add_interval(now,interval(1d)) - now = interval(1d), but duration(MS_IN_DAY) != interval(1d) because we may cross a (calendar) offset, either due to a leap second, leap hour (daylight, which I forgot about in the comment above), leap day (February), etc.

[houqp]

Good summary 👍 this aligns with what I have in mind as well.

[emkornfield]

Could you send an e-mail out to dev@ to make sure this gets wide visibility

[jorgecarleitao]

Updating the summary table above with the result of the follow-up discussion,

care about leap	Type
No	Duration
hours	Interval DaysMs
days and hours	Interval MonthsDaysNs
days, hours, and seconds	Extension

[houqp]

I removed sentences that mentioned ordering and leap days. now the only change is mention of leap seconds to be consistent with other temporal types.

[pitrou]

+1. This is fine with me.

[houqp]

@edponce pushed a new commit to apply your suggestions, PTAL.

[wesm]

+1