Re: Have I found an interval arithmetic bug?

On Fri, Apr 2, 2021 at 01:05:42PM -0700, Bryn Llewellyn wrote:
> I should come clean about the larger context. I work for Yugabyte, Inc. We have
> a distributed SQL database that uses the Version 11.2 PostgreSQL C code for SQL
> processing “as is”.
>
> https://blog.yugabyte.com/
> distributed-postgresql-on-a-google-spanner-architecture-query-layer/
>
> The founders decided to document YugabyteDB’s SQL functionality explicitly
> rather than just to point to the published PostgreSQL doc. (There are some DDL
> differences that reflect the storage layer differences.) I’m presently
> documenting date-time functionality. This is why I’m so focused on
> understanding the semantics exactly and on understanding the requirements that
> the functionality was designed to meet. I’m struggling with interval
> functionality. I read this:

[Sorry, also moved this to hackers. I might normally do all the
discussion on general, with patches, and then move it to hackers, but
our PG 14 deadline is next week, so it is best to move it now in hopes
it can be addressed in PG 14.]

Sure, seems like a good idea.

> https://www.postgresql.org/docs/current/datatype-datetime.html#
> DATATYPE-INTERVAL-INPUT
>
> « …field values can have fractional parts; for example '1.5 week' or
> '01:02:03.45'. Such input is converted to the appropriate number of months,
> days, and seconds for storage. When this would result in a fractional number of
> months or days, the fraction is added to the lower-order fields using the
> conversion factors 1 month = 30 days and 1 day = 24 hours. For example, '1.5
> month' becomes 1 month and 15 days. Only seconds will ever be shown as
> fractional on output. »
>
> Notice that the doc says that spill-down goes all the way to seconds and not
> just one unit. This simple test is consistent with the doc (output follows the
> dash-dash comment):
>
> select ('6.54321 months'::interval)::text as i; -- 6 mons 16 days 07:06:40.32
>
> You see similar spill-down with this:
>
> select ('876.54321 days'::interval)::text as i; -- 876 days 13:02:13.344
>
> And so on down through the remaining smaller units. It’s only this test that
> doesn’t spill down one unit:
>
> select ('6.54321 years'::interval)::text as i; -- 6 years 6 mons
>
> This does suggest a straight bug rather than a case for committee debate about
> what might have been intended. What do you think, Bruce?

So, that gets into more detail. When I said "spill down one unit", I
was not talking about _visible_ units, but rather the three internal
units used by Postgres:

https://www.postgresql.org/docs/13/datatype-datetime.html#DATATYPE-INTERVAL-INPUT
Internally interval values are stored as months, days, and seconds.
-------------------------

However, while that explains why years don't spill beyond months, it
doesn't explain why months would spill beyond days. This certainly
seems inconsistent.

I have modified the patch to prevent partial months from creating
partial hours/minutes/seconds, so the output is now at least consistent
based on the three units:

SELECT ('6.54321 years'::interval)::text as i;
i
----------------
6 years 7 mons

SELECT ('6.54321 months'::interval)::text as i;
i
----------------
6 mons 16 days

SELECT ('876.54321 days'::interval)::text as i;
i
-----------------------
876 days 13:02:13.344

Partial years keeps it in months, partial months takes it to days, and
partial days take it to hours/minutes/seconds. This seems like an
improvement.

This also changes the regression test output, I think for the better:

SELECT INTERVAL '1.5 weeks';
i
------------------
- 10 days 12:00:00
+ 10 days

The new output is less precise, but probably closer to what the user
wanted.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

If only the physical world exists, free will is an illusion.