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 <br...@momjian.us> https://momjian.us
EDB https://enterprisedb.com
If only the physical world exists, free will is an illusion.
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 889077f55c..d5b3705145 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -526,7 +526,6 @@ AdjustFractDays(double frac, struct pg_tm *tm, fsec_t *fsec, int scale)
extra_days = (int) frac;
tm->tm_mday += extra_days;
frac -= extra_days;
- AdjustFractSeconds(frac, tm, fsec, SECS_PER_DAY);
}
/* Fetch a fractional-second value with suitable error checking */
@@ -3307,28 +3306,28 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
case DTK_YEAR:
tm->tm_year += val;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
tmask = DTK_M(YEAR);
break;
case DTK_DECADE:
tm->tm_year += val * 10;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 10;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 10);
tmask = DTK_M(DECADE);
break;
case DTK_CENTURY:
tm->tm_year += val * 100;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 100;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 100);
tmask = DTK_M(CENTURY);
break;
case DTK_MILLENNIUM:
tm->tm_year += val * 1000;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 1000;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 1000);
tmask = DTK_M(MILLENNIUM);
break;
@@ -3565,7 +3564,7 @@ DecodeISO8601Interval(char *str,
{
case 'Y':
tm->tm_year += val;
- tm->tm_mon += (fval * MONTHS_PER_YEAR);
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
break;
case 'M':
tm->tm_mon += val;
@@ -3601,7 +3600,7 @@ DecodeISO8601Interval(char *str,
return DTERR_BAD_FORMAT;
tm->tm_year += val;
- tm->tm_mon += (fval * MONTHS_PER_YEAR);
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
if (unit == '\0')
return 0;
if (unit == 'T')
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c
index 4245016c8e..d1bb00db01 100644
--- a/src/interfaces/ecpg/pgtypeslib/interval.c
+++ b/src/interfaces/ecpg/pgtypeslib/interval.c
@@ -529,28 +529,28 @@ DecodeInterval(char **field, int *ftype, int nf, /* int range, */
case DTK_YEAR:
tm->tm_year += val;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
case DTK_DECADE:
tm->tm_year += val * 10;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 10;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 10);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
case DTK_CENTURY:
tm->tm_year += val * 100;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 100;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 100);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
case DTK_MILLENNIUM:
tm->tm_year += val * 1000;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 1000;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 1000);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index c5ffa9f2cc..bd35c930b0 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -34,10 +34,10 @@ SELECT INTERVAL '-1 days +02:03' AS "22 hours ago...";
-1 day +02:03:00
(1 row)
-SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
- Ten days twelve hours
------------------------
- 10 days 12:00:00
+SELECT INTERVAL '1.5 weeks' AS "Ten days";
+ Ten days
+----------
+ 10 days
(1 row)
SELECT INTERVAL '1.5 months' AS "One month 15 days";
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 11c1929bef..bd141e7b52 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -11,7 +11,7 @@ SELECT INTERVAL '+02:00' AS "Two hours";
SELECT INTERVAL '-08:00' AS "Eight hours";
SELECT INTERVAL '-1 +02:03' AS "22 hours ago...";
SELECT INTERVAL '-1 days +02:03' AS "22 hours ago...";
-SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
+SELECT INTERVAL '1.5 weeks' AS "Ten days";
SELECT INTERVAL '1.5 months' AS "One month 15 days";
SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
