Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m9Xmx-0007Vq-Ju for pgsql-hackers@arkaria.postgresql.org; Fri, 30 Jul 2021 19:03:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1m9Xmw-0001iy-K8 for pgsql-hackers@arkaria.postgresql.org; Fri, 30 Jul 2021 19:03:18 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m9Xmw-0001iq-Ay for pgsql-hackers@lists.postgresql.org; Fri, 30 Jul 2021 19:03:18 +0000 Received: from momjian.us ([72.94.173.45]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m9Xmt-0001rW-IU for pgsql-hackers@lists.postgresql.org; Fri, 30 Jul 2021 19:03:17 +0000 Received: from bruce by momjian.us with local (Exim 4.92) (envelope-from ) id 1m9Xmr-0007T6-JR; Fri, 30 Jul 2021 15:03:13 -0400 Date: Fri, 30 Jul 2021 15:03:13 -0400 From: Bruce Momjian To: Tom Lane Cc: Dean Rasheed , John W Higgins , pgsql-hackers list Subject: Re: Have I found an interval arithmetic bug? Message-ID: <20210730190313.GC9600@momjian.us> References: <20210727193614.GE19774@momjian.us> <456756.1627416114@sss.pgh.pa.us> <20210727211314.GA23040@momjian.us> <901454CE-2C0D-4C49-ADAB-A094C03B7CB1@yugabyte.com> <20210728151916.GA15440@momjian.us> <20210730160439.GZ9600@momjian.us> <20210730160854.GA9600@momjian.us> <1008623.1627663774@sss.pgh.pa.us> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="rwbb4r/vLufKlfJs" Content-Disposition: inline In-Reply-To: <1008623.1627663774@sss.pgh.pa.us> User-Agent: Mutt/1.10.1 (2018-07-13) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --rwbb4r/vLufKlfJs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Jul 30, 2021 at 12:49:34PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > Now that I think of it, I will just remove the word "rounded" from the > > back branch docs so we are technically breaking the documented API less > > in PG 15. > > I think your first idea was better. Not documenting the behavior > doesn't make this not an API change; it just makes it harder for > people to understand what changed. OK. However, I thought we were more worried about changing documented APIs than undocumented ones. Anyway, I will do as you suggested. > The doc patch itself is not exactly fine: > > + Field values can have fractional parts; for example, '1.5 > + weeks' or '01:02:03.45'. However, > > I think "some field values", as it was worded previously, was better. > If you try to write 01.5:02:03, that is not going to be interpreted > as 1.5 hours. (Hmm, I get something that seems quite insane: > > regression=# select '01.5:02:03'::interval; > interval > ---------------- > 1 day 14:03:00 > (1 row) > > I wonder what it thinks it's doing there.) It thinks 01.5:02:03 is Days:Hours;Minute, so I think all fields can use fractions: SELECT interval '1.5 minutes'; interval ---------- 00:01:30 > This is wrong: > > + because interval internally stores only three integer units (months, > + days, seconds), fractional units must be spilled to smaller units. > > s/seconds/microseconds/ is probably enough to fix that. OK, there were a few place that said "seconds" so I fixed those too. > + For example, because months are approximated to equal 30 days, > + fractional values of units greater than months is rounded to be the > + nearest integer number of months. Fractional units of months or less > + are computed to be an integer number of days and seconds, assuming > + 24 hours per day. For example, '1.5 months' > + becomes 1 month 15 days. > > This entire passage is vague, and grammatically shaky too. Perhaps > more like > > Fractional parts of units larger than months are rounded to the > nearest integer number of months; for example '1.5 years' > becomes '1 year 6 mons'. Fractional parts of months are rounded > to the nearest integer number of days, using the assumption that > one month equals 30 days; for example '1.5 months' The newest patch actually doesn't work as explained above --- fractional months now continue to spill to microseconds. I think you are looking at a previous version. > becomes '1 mon 15 days'. Fractional parts of days and weeks > are converted to microseconds, using the assumption that one day > equals 24 hours. Uh, fractional weeks can be integer days. > On output, the months field is shown as an appropriate number of > years and months; the days field is shown as-is; the microseconds > field is converted to hours, minutes, and possibly-fractional > seconds. Here is an updated patch that includes some of your ideas. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion. --rwbb4r/vLufKlfJs Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="interval.diff" diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index 453115f942..50a2c8e5f1 100644 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -2840,15 +2840,18 @@ P years-months- - In the verbose input format, and in some fields of the more compact - input formats, 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. + Field values can have fractional parts: for example, '1.5 + weeks' or '01:02:03.45'. However, + because interval internally stores only three integer units (months, + days, microseconds), fractional units must be spilled to smaller + units. Fractional parts of units greater than months is rounded to + be an integer number of months, e.g. '1.5 years' + becomes '1 year 6 mons'. Fractional parts of + weeks and days are computed to be an integer number of days and + microseconds, assuming 30 days per month and 24 hours per day, e.g., + '1.75 months' becomes 1 mon 22 days + 12:00:00. Only seconds will ever be shown as fractional + on output. @@ -2892,10 +2895,10 @@ P years-months- Internally interval values are stored as months, days, - and seconds. This is done because the number of days in a month + and microseconds. This is done because the number of days in a month varies, and a day can have 23 or 25 hours if a daylight savings time adjustment is involved. The months and days fields are integers - while the seconds field can store fractions. Because intervals are + while the microseconds field can store fractional seconds. Because intervals are usually created from constant strings or timestamp subtraction, this storage method works well in most cases, but can cause unexpected results: diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c index 54ae632de2..cb3fa85892 100644 --- a/src/backend/utils/adt/datetime.c +++ b/src/backend/utils/adt/datetime.c @@ -3306,29 +3306,25 @@ 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 +3561,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 +3597,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 02b3c47223..a7e530cb5d 100644 --- a/src/interfaces/ecpg/pgtypeslib/interval.c +++ b/src/interfaces/ecpg/pgtypeslib/interval.c @@ -155,7 +155,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; @@ -191,7 +191,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') @@ -528,29 +528,25 @@ 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; --rwbb4r/vLufKlfJs--