On Mon, Apr  5, 2021 at 11:33:10AM -0500, Justin Pryzby wrote:
> On Fri, Apr 02, 2021 at 10:21:26PM -0400, Bruce Momjian wrote:
> > I wish I could figure out how to improve it any futher.  What is odd is
> > that I have never seen this reported as a problem before.  I plan to
> > apply this early next week for PG 14.
> 
> On Fri, Apr 02, 2021 at 01:05:42PM -0700, Bryn Llewellyn wrote:
> > br...@momjian.us wrote:
> > > Yes, looking at the code, it seems we only spill down to one unit, not 
> > > more. I think we need to have a discussion if we want to change that. 
> 
> If this is a bug, then there's no deadline - and if you're proposing a 
> behavior
> change, then I don't think it's a good time to begin the discussion.

Well, bug or not, we are not going to change back branches for this, and
if you want a larger discussion, it will have to wait for PG 15.

> > 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. »

I see that.  What is not clear here is how far we flow down.  I was
looking at adding documentation or regression tests for that, but was
unsure.  I adjusted the docs slightly in the attached patch.

> Your patch changes what seems to be the intended behavior, with the test case
> added by:
> 
> |commit 57bfb27e60055c10e42b87e68a894720c2f78e70
> |Author: Tom Lane <t...@sss.pgh.pa.us>
> |Date:   Mon Sep 4 01:26:28 2006 +0000
> |
> |    Fix interval input parser so that fractional weeks and months are
> |    cascaded first to days and only what is leftover into seconds.  This
> 
> And documented by:
> 
> |commit dbf57d31f8d7bf5c058a9fab2a1ccb4a336864ce
> |Author: Tom Lane <t...@sss.pgh.pa.us>
> |Date:   Sun Nov 9 17:09:48 2008 +0000
> |
> |    Add some documentation about handling of fractions in interval input.
> |    (It's always worked like this, but we never documented it before.)
> 
> If you were to change the behavior, I think you'd have to update the
> documentation, too - but I think that's not a desirable change.

> I *am* curious why the YEAR, DECADE, CENTURY, AND MILLENIUM cases only handle
> fractional intervals down to the next smaller unit, and not down to
> seconds/milliseconds.  I wrote a patch to handle that by adding
> AdjustFractMons(), if we agree that it's desirable to change.

The interaction of months/days/seconds is so imprecise that passing it
futher down doesn't make much sense, and suggests a precision that
doesn't exist, but if people prefer that we can do it.

-- 
  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/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index 7c341c8e3f..6b50fb849f 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -2775,7 +2775,7 @@ P <optional> <replaceable>years</replaceable>-<replaceable>months</replaceable>-
      <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
+     months or days, the fraction is added to the next lower-order internal field
      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.
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..fccb9765ae 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')
@@ -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...";
 

Reply via email to