From: Bruce Momjian <bruce@momjian.us>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: John W Higgins <wishdev@gmail.com>
Cc: pgsql-hackers list <pgsql-hackers@lists.postgresql.org>
Subject: Re: Have I found an interval arithmetic bug?
Date: Fri, 30 Jul 2021 15:03:13 -0400
Message-ID: <20210730190313.GC9600@momjian.us> (raw)
In-Reply-To: <1008623.1627663774@sss.pgh.pa.us>
References: <20210727193614.GE19774@momjian.us>
<456756.1627416114@sss.pgh.pa.us>
<20210727211314.GA23040@momjian.us>
<901454CE-2C0D-4C49-ADAB-A094C03B7CB1@yugabyte.com>
<CAPhAwGyccJZT-mXzHU1216Q-Z_xY3q+4TixzZ2sZf3hiPFe5tg@mail.gmail.com>
<CAEZATCWaK0qdTiqxS=fM03VaKcfbYmSOV_JzqYDrVn2nSaMnFQ@mail.gmail.com>
<20210728151916.GA15440@momjian.us>
<20210730160439.GZ9600@momjian.us>
<20210730160854.GA9600@momjian.us>
<1008623.1627663774@sss.pgh.pa.us>
On Fri, Jul 30, 2021 at 12:49:34PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> 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, <literal>'1.5
> + weeks'</literal> or <literal>'01:02:03.45'</literal>. 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, <literal>'1.5 months'</literal>
> + becomes <literal>1 month 15 days</literal>.
>
> 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 <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
If only the physical world exists, free will is an illusion.
Attachments:interval.diff (text/x-diff, 6.1K) downloaddiff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgmlindex 453115f942..50a2c8e5f1 100644--- a/doc/src/sgml/datatype.sgml+++ b/doc/src/sgml/datatype.sgml@@ -2840,15 +2840,18 @@ P <optional> <replaceable>years</replaceable>-<replaceable>months</replaceable>-
</para>
<para>
- In the verbose input format, and in some fields of the more compact- input formats, field values can have fractional parts; for example- <literal>'1.5 week'</literal> or <literal>'01:02:03.45'</literal>. 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, <literal>'1.5 month'</literal> becomes 1 month and 15 days.- Only seconds will ever be shown as fractional on output.+ Field values can have fractional parts: for example, <literal>'1.5+ weeks'</literal> or <literal>'01:02:03.45'</literal>. 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. <literal>'1.5 years'</literal>+ becomes <literal>'1 year 6 mons'</literal>. 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.,+ <literal>'1.75 months'</literal> becomes <literal>1 mon 22 days+ 12:00:00</literal>. Only seconds will ever be shown as fractional+ on output.
</para>
<para>
@@ -2892,10 +2895,10 @@ P <optional> <replaceable>years</replaceable>-<replaceable>months</replaceable>-
<para>
Internally <type>interval</type> 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 <type>timestamp</type> 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.cindex 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.cindex 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;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bruce@momjian.us, tgl@sss.pgh.pa.us, dean.a.rasheed@gmail.com, wishdev@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Have I found an interval arithmetic bug?
In-Reply-To: <20210730190313.GC9600@momjian.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox