public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v29 03/11] Allow to prolong life span of transition tables until transaction end
20+ messages / 3 participants
[nested] [flat]

* [PATCH v29 03/11] Allow to prolong life span of transition tables until transaction end
@ 2019-12-20 01:09 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Yugo Nagata @ 2019-12-20 01:09 UTC (permalink / raw)

Originally, tuplestores of AFTER trigger's transition tables were
freed for each query depth. For our IVM implementation, we would like
to prolong life of the tuplestores because we have to preserve them
for a whole query assuming that some base tables might be changed
in some trigger functions.
---
 src/backend/commands/trigger.c | 83 ++++++++++++++++++++++++++++++++--
 src/include/commands/trigger.h |  2 +
 2 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 52177759ab..00b20f4b5b 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3742,6 +3742,10 @@ typedef struct AfterTriggerEventList
  * end of the list, so it is relatively easy to discard them.  The event
  * list chunks themselves are stored in event_cxt.
  *
+ * prolonged_tuplestored is a list of transition table tuplestores whose
+ * life are prolonged to the end of the outmost query instead of each nested
+ * query.
+ *
  * query_depth is the current depth of nested AfterTriggerBeginQuery calls
  * (-1 when the stack is empty).
  *
@@ -3807,6 +3811,7 @@ typedef struct AfterTriggersData
 	SetConstraintState state;	/* the active S C state */
 	AfterTriggerEventList events;	/* deferred-event list */
 	MemoryContext event_cxt;	/* memory context for events, if any */
+	List   *prolonged_tuplestores;	/* list of prolonged tuplestores */
 
 	/* per-query-level data: */
 	AfterTriggersQueryData *query_stack;	/* array of structs shown below */
@@ -3842,6 +3847,7 @@ struct AfterTriggersTableData
 	bool		closed;			/* true when no longer OK to add tuples */
 	bool		before_trig_done;	/* did we already queue BS triggers? */
 	bool		after_trig_done;	/* did we already queue AS triggers? */
+	bool		prolonged;			/* are transition tables prolonged? */
 	AfterTriggerEventList after_trig_events;	/* if so, saved list pointer */
 
 	/*
@@ -3891,6 +3897,7 @@ static void TransitionTableAddTuple(EState *estate,
 									TupleTableSlot *original_insert_tuple,
 									Tuplestorestate *tuplestore);
 static void AfterTriggerFreeQuery(AfterTriggersQueryData *qs);
+static void release_or_prolong_tuplestore(Tuplestorestate *ts, bool prolonged);
 static SetConstraintState SetConstraintStateCreate(int numalloc);
 static SetConstraintState SetConstraintStateCopy(SetConstraintState origstate);
 static SetConstraintState SetConstraintStateAddItem(SetConstraintState state,
@@ -4768,6 +4775,45 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
 }
 
 
+/*
+ * SetTransitionTablePreserved
+ *
+ * Prolong lifespan of transition tables corresponding specified relid and
+ * command type to the end of the outmost query instead of each nested query.
+ * This enables to use nested AFTER trigger's transition tables from outer
+ * query's triggers.  Currently, only immediate incremental view maintenance
+ * uses this.
+ */
+void
+SetTransitionTablePreserved(Oid relid, CmdType cmdType)
+{
+	AfterTriggersTableData *table;
+	AfterTriggersQueryData *qs;
+	bool		found = false;
+	ListCell   *lc;
+
+	/* Check state, like AfterTriggerSaveEvent. */
+	if (afterTriggers.query_depth < 0)
+		elog(ERROR, "SetTransitionTablePreserved() called outside of query");
+
+	qs = &afterTriggers.query_stack[afterTriggers.query_depth];
+
+	foreach(lc, qs->tables)
+	{
+		table = (AfterTriggersTableData *) lfirst(lc);
+		if (table->relid == relid && table->cmdType == cmdType &&
+			table->closed)
+		{
+			table->prolonged = true;
+			found = true;
+		}
+	}
+
+	if (!found)
+		elog(ERROR,"could not find table with OID %d and command type %d", relid, cmdType);
+}
+
+
 /*
  * GetAfterTriggersTableData
  *
@@ -4978,6 +5024,7 @@ AfterTriggerBeginXact(void)
 	 */
 	afterTriggers.firing_counter = (CommandId) 1;	/* mustn't be 0 */
 	afterTriggers.query_depth = -1;
+	afterTriggers.prolonged_tuplestores = NIL;
 
 	/*
 	 * Verify that there is no leftover state remaining.  If these assertions
@@ -5138,19 +5185,19 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
 		ts = table->old_upd_tuplestore;
 		table->old_upd_tuplestore = NULL;
 		if (ts)
-			tuplestore_end(ts);
+			release_or_prolong_tuplestore(ts, table->prolonged);
 		ts = table->new_upd_tuplestore;
 		table->new_upd_tuplestore = NULL;
 		if (ts)
-			tuplestore_end(ts);
+			release_or_prolong_tuplestore(ts, table->prolonged);
 		ts = table->old_del_tuplestore;
 		table->old_del_tuplestore = NULL;
 		if (ts)
-			tuplestore_end(ts);
+			release_or_prolong_tuplestore(ts, table->prolonged);
 		ts = table->new_ins_tuplestore;
 		table->new_ins_tuplestore = NULL;
 		if (ts)
-			tuplestore_end(ts);
+			release_or_prolong_tuplestore(ts, table->prolonged);
 		if (table->storeslot)
 		{
 			TupleTableSlot *slot = table->storeslot;
@@ -5167,6 +5214,34 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
 	 */
 	qs->tables = NIL;
 	list_free_deep(tables);
+
+	/* Release prolonged tuplestores at the end of the outmost query */
+	if (afterTriggers.query_depth == 0)
+	{
+		foreach(lc, afterTriggers.prolonged_tuplestores)
+		{
+			ts = (Tuplestorestate *) lfirst(lc);
+			if (ts)
+				tuplestore_end(ts);
+		}
+		afterTriggers.prolonged_tuplestores = NIL;
+	}
+}
+
+/*
+ * Release the tuplestore, or append it to the prolonged tuplestores list.
+ */
+static void
+release_or_prolong_tuplestore(Tuplestorestate *ts, bool prolonged)
+{
+	if (prolonged && afterTriggers.query_depth > 0)
+	{
+		MemoryContext oldcxt = MemoryContextSwitchTo(CurTransactionContext);
+		afterTriggers.prolonged_tuplestores = lappend(afterTriggers.prolonged_tuplestores, ts);
+		MemoryContextSwitchTo(oldcxt);
+	}
+	else
+		tuplestore_end(ts);
 }
 
 
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 430e3ca7dd..48a21c4c51 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -265,6 +265,8 @@ extern void AfterTriggerEndSubXact(bool isCommit);
 extern void AfterTriggerSetState(ConstraintsSetStmt *stmt);
 extern bool AfterTriggerPendingOnRel(Oid relid);
 
+extern void SetTransitionTablePreserved(Oid relid, CmdType cmdType);
+
 
 /*
  * in utils/adt/ri_triggers.c
-- 
2.25.1


--Multipart=_Mon__28_Aug_2023_11_52_52_+0900_hj6L5h176QaSGtg7
Content-Type: text/x-diff;
 name="v29-0004-Add-Incremental-View-Maintenance-support-to-pg_d.patch"
Content-Disposition: attachment;
 filename="v29-0004-Add-Incremental-View-Maintenance-support-to-pg_d.patch"
Content-Transfer-Encoding: 7bit



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
@ 2022-04-02 00:06 Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-02 00:06 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

Joseph Koshakow <[email protected]> writes:
> * The existing code for rounding had a lot of int to double
> casting and vice versa. I *think* that doubles are able to completely
> represent the range of ints. However doubles are not able to represent
> the full range of int64. After making the change I started noticing
> a lot of lossy behavior. One thought I had was to change the doubles
> to long doubles, but I wasn't able to figure out if long doubles could
> completely represent the range of int64. Especially since their size
> varies depending on the architecture. Does anyone know the answer to
> this?

I agree that relying on long double is not a great plan.  However,
I'm not seeing where there's a problem.  AFAICS the revised code
only uses doubles to represent fractions from the input, ie if you
write "123.456 hours" then the ".456" is carried around for awhile
as a float.  This does not seem likely to pose any real-world
problem; do you have a counterexample?

Anyway, I've spent today reviewing the code and cleaning up things
I didn't like, and attached is a v10.  I almost feel that this is
committable, but there is one thing that is bothering me.  The
part of DecodeInterval that does strange things with signs in the
INTSTYLE_SQL_STANDARD case (starting about line 3400 in datetime.c
before this patch, or line 3600 after) used to separately force the
hour, minute, second, and microsecond fields to negative.
Now it forces the merged tm_usec field to negative.  It seems to
me that this could give a different answer than before, if the
h/m/s/us values had been of different signs before they got merged.
However, I don't think that that situation is possible in SQL-spec-
compliant input, so it may not be a problem.  Again, a counterexample
would be interesting.

			regards, tom lane



Attachments:

  [text/x-diff] v10-0001-Check-for-overflow-when-decoding-an-interval.patch (93.2K, ../../[email protected]/2-v10-0001-Check-for-overflow-when-decoding-an-interval.patch)
  download | inline diff:
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index ba0ec35ac5..dae90e4a9e 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -21,6 +21,7 @@
 #include "access/htup_details.h"
 #include "access/xact.h"
 #include "catalog/pg_type.h"
+#include "common/int.h"
 #include "common/string.h"
 #include "funcapi.h"
 #include "miscadmin.h"
@@ -37,17 +38,31 @@ static int	DecodeNumber(int flen, char *field, bool haveTextMonth,
 static int	DecodeNumberField(int len, char *str,
 							  int fmask, int *tmask,
 							  struct pg_tm *tm, fsec_t *fsec, bool *is2digits);
+static int	DecodeTimeCommon(char *str, int fmask, int range,
+							 int *tmask, struct pg_itm *itm);
 static int	DecodeTime(char *str, int fmask, int range,
 					   int *tmask, struct pg_tm *tm, fsec_t *fsec);
+static int	DecodeTimeForInterval(char *str, int fmask, int range,
+								  int *tmask, struct pg_itm_in *itm_in);
 static const datetkn *datebsearch(const char *key, const datetkn *base, int nel);
 static int	DecodeDate(char *str, int fmask, int *tmask, bool *is2digits,
 					   struct pg_tm *tm);
 static char *AppendSeconds(char *cp, int sec, fsec_t fsec,
 						   int precision, bool fillzeros);
-static void AdjustFractSeconds(double frac, struct pg_tm *tm, fsec_t *fsec,
-							   int scale);
-static void AdjustFractDays(double frac, struct pg_tm *tm, fsec_t *fsec,
-							int scale);
+static bool int64_multiply_add(int64 val, int64 multiplier, int64 *sum);
+static bool AdjustFractMicroseconds(double frac, int64 scale,
+									struct pg_itm_in *itm_in);
+static bool AdjustFractDays(double frac, int scale,
+							struct pg_itm_in *itm_in);
+static bool AdjustFractYears(double frac, int scale,
+							 struct pg_itm_in *itm_in);
+static bool AdjustMicroseconds(int64 val, double fval, int64 scale,
+							   struct pg_itm_in *itm_in);
+static bool AdjustDays(int64 val, int scale,
+					   struct pg_itm_in *itm_in);
+static bool AdjustMonths(int64 val, struct pg_itm_in *itm_in);
+static bool AdjustYears(int64 val, int scale,
+						struct pg_itm_in *itm_in);
 static int	DetermineTimeZoneOffsetInternal(struct pg_tm *tm, pg_tz *tzp,
 											pg_time_t *tp);
 static bool DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t,
@@ -425,7 +440,7 @@ GetCurrentTimeUsec(struct pg_tm *tm, fsec_t *fsec, int *tzp)
  * Returns a pointer to the new end of string.  No NUL terminator is put
  * there; callers are responsible for NUL terminating str themselves.
  *
- * Note that any sign is stripped from the input seconds values.
+ * Note that any sign is stripped from the input sec and fsec values.
  */
 static char *
 AppendSeconds(char *cp, int sec, fsec_t fsec, int precision, bool fillzeros)
@@ -471,7 +486,7 @@ AppendSeconds(char *cp, int sec, fsec_t fsec, int precision, bool fillzeros)
 
 		/*
 		 * If we still have a non-zero value then precision must have not been
-		 * enough to print the number.  We punt the problem to pg_ltostr(),
+		 * enough to print the number.  We punt the problem to pg_ultostr(),
 		 * which will generate a correct answer in the minimum valid width.
 		 */
 		if (value)
@@ -496,39 +511,163 @@ AppendTimestampSeconds(char *cp, struct pg_tm *tm, fsec_t fsec)
 	return AppendSeconds(cp, tm->tm_sec, fsec, MAX_TIMESTAMP_PRECISION, true);
 }
 
+
+/*
+ * Add val * multiplier to *sum.
+ * Returns true if successful, false on overflow.
+ */
+static bool
+int64_multiply_add(int64 val, int64 multiplier, int64 *sum)
+{
+	int64		product;
+
+	if (pg_mul_s64_overflow(val, multiplier, &product) ||
+		pg_add_s64_overflow(*sum, product, sum))
+		return false;
+	return true;
+}
+
 /*
- * Multiply frac by scale (to produce seconds) and add to *tm & *fsec.
- * We assume the input frac is less than 1 so overflow is not an issue.
+ * Multiply frac by scale (to produce microseconds) and add to itm_in->tm_usec.
+ * Returns true if successful, false if itm_in overflows.
  */
-static void
-AdjustFractSeconds(double frac, struct pg_tm *tm, fsec_t *fsec, int scale)
+static bool
+AdjustFractMicroseconds(double frac, int64 scale,
+						struct pg_itm_in *itm_in)
 {
-	int			sec;
+	int64		usec;
 
+	/* Fast path for common case */
 	if (frac == 0)
-		return;
+		return true;
+
+	/*
+	 * We assume the input frac is less than 1, so overflow of frac or usec is
+	 * not an issue for interesting values of scale.
+	 */
 	frac *= scale;
-	sec = (int) frac;
-	tm->tm_sec += sec;
-	frac -= sec;
-	*fsec += rint(frac * 1000000);
+	usec = (int64) frac;
+
+	/* Round off any fractional microsecond */
+	frac -= usec;
+	if (frac > 0.5)
+		usec++;
+	else if (frac < -0.5)
+		usec--;
+
+	return !pg_add_s64_overflow(itm_in->tm_usec, usec, &itm_in->tm_usec);
 }
 
-/* As above, but initial scale produces days */
-static void
-AdjustFractDays(double frac, struct pg_tm *tm, fsec_t *fsec, int scale)
+/*
+ * Multiply frac by scale (to produce days).  Add the integral part of the
+ * result to itm_in->tm_mday, the fractional part to itm_in->tm_usec.
+ * Returns true if successful, false if itm_in overflows.
+ */
+static bool
+AdjustFractDays(double frac, int scale,
+				struct pg_itm_in *itm_in)
 {
 	int			extra_days;
 
+	/* Fast path for common case */
 	if (frac == 0)
-		return;
+		return true;
+
+	/*
+	 * We assume the input frac is less than 1, so overflow of frac or
+	 * extra_days is not an issue.
+	 */
 	frac *= scale;
 	extra_days = (int) frac;
-	tm->tm_mday += extra_days;
+
+	/* ... but this could overflow, if tm_mday is already nonzero */
+	if (pg_add_s32_overflow(itm_in->tm_mday, extra_days, &itm_in->tm_mday))
+		return false;
+
+	/* Handle any fractional day */
 	frac -= extra_days;
-	AdjustFractSeconds(frac, tm, fsec, SECS_PER_DAY);
+	return AdjustFractMicroseconds(frac, USECS_PER_DAY, itm_in);
 }
 
+/*
+ * Multiply frac by scale (to produce years), then further scale up to months.
+ * Add the integral part of the result to itm_in->tm_mon, discarding any
+ * fractional part.
+ * Returns true if successful, false if itm_in overflows.
+ */
+static bool
+AdjustFractYears(double frac, int scale,
+				 struct pg_itm_in *itm_in)
+{
+	/*
+	 * As above, we assume abs(frac) < 1, so this can't overflow for any
+	 * interesting value of scale.
+	 */
+	int			extra_months = (int) rint(frac * scale * MONTHS_PER_YEAR);
+
+	return !pg_add_s32_overflow(itm_in->tm_mon, extra_months, &itm_in->tm_mon);
+}
+
+/*
+ * Add (val + fval) * scale to itm_in->tm_usec.
+ * Returns true if successful, false if itm_in overflows.
+ */
+static bool
+AdjustMicroseconds(int64 val, double fval, int64 scale,
+				   struct pg_itm_in *itm_in)
+{
+	/* Handle the integer part */
+	if (!int64_multiply_add(val, scale, &itm_in->tm_usec))
+		return false;
+	/* Handle the float part */
+	return AdjustFractMicroseconds(fval, scale, itm_in);
+}
+
+/*
+ * Multiply val by scale (to produce days) and add to itm_in->tm_mday.
+ * Returns true if successful, false if itm_in overflows.
+ */
+static bool
+AdjustDays(int64 val, int scale, struct pg_itm_in *itm_in)
+{
+	int			days;
+
+	if (val < INT_MIN || val > INT_MAX)
+		return false;
+	return !pg_mul_s32_overflow((int32) val, scale, &days) &&
+		!pg_add_s32_overflow(itm_in->tm_mday, days, &itm_in->tm_mday);
+}
+
+/*
+ * Add val to itm_in->tm_mon (no need for scale here, as val is always
+ * in months already).
+ * Returns true if successful, false if itm_in overflows.
+ */
+static bool
+AdjustMonths(int64 val, struct pg_itm_in *itm_in)
+{
+	if (val < INT_MIN || val > INT_MAX)
+		return false;
+	return !pg_add_s32_overflow(itm_in->tm_mon, (int32) val, &itm_in->tm_mon);
+}
+
+/*
+ * Multiply val by scale (to produce years) and add to itm_in->tm_year.
+ * Returns true if successful, false if itm_in overflows.
+ */
+static bool
+AdjustYears(int64 val, int scale,
+			struct pg_itm_in *itm_in)
+{
+	int			years;
+
+	if (val < INT_MIN || val > INT_MAX)
+		return false;
+	return !pg_mul_s32_overflow((int32) val, scale, &years) &&
+		!pg_add_s32_overflow(itm_in->tm_year, years, &itm_in->tm_year);
+}
+
+
 /* Fetch a fractional-second value with suitable error checking */
 static int
 ParseFractionalSecond(char *cp, fsec_t *fsec)
@@ -2548,79 +2687,143 @@ ValidateDate(int fmask, bool isjulian, bool is2digits, bool bc,
 }
 
 
-/* DecodeTime()
+/* DecodeTimeCommon()
  * Decode time string which includes delimiters.
  * Return 0 if okay, a DTERR code if not.
+ * tmask and itm are output parameters.
  *
- * Only check the lower limit on hours, since this same code can be
- * used to represent time spans.
+ * This code is shared between the timestamp and interval cases.
+ * We return a struct pg_itm (of which only the tm_usec, tm_sec, tm_min,
+ * and tm_hour fields are used) and let the wrapper functions below
+ * convert and range-check as necessary.
  */
 static int
-DecodeTime(char *str, int fmask, int range,
-		   int *tmask, struct pg_tm *tm, fsec_t *fsec)
+DecodeTimeCommon(char *str, int fmask, int range,
+				 int *tmask, struct pg_itm *itm)
 {
 	char	   *cp;
 	int			dterr;
+	fsec_t		fsec = 0;
 
 	*tmask = DTK_TIME_M;
 
 	errno = 0;
-	tm->tm_hour = strtoint(str, &cp, 10);
+	itm->tm_hour = strtoi64(str, &cp, 10);
 	if (errno == ERANGE)
 		return DTERR_FIELD_OVERFLOW;
 	if (*cp != ':')
 		return DTERR_BAD_FORMAT;
 	errno = 0;
-	tm->tm_min = strtoint(cp + 1, &cp, 10);
+	itm->tm_min = strtoint(cp + 1, &cp, 10);
 	if (errno == ERANGE)
 		return DTERR_FIELD_OVERFLOW;
 	if (*cp == '\0')
 	{
-		tm->tm_sec = 0;
-		*fsec = 0;
+		itm->tm_sec = 0;
 		/* If it's a MINUTE TO SECOND interval, take 2 fields as being mm:ss */
 		if (range == (INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND)))
 		{
-			tm->tm_sec = tm->tm_min;
-			tm->tm_min = tm->tm_hour;
-			tm->tm_hour = 0;
+			if (itm->tm_hour > INT_MAX || itm->tm_hour < INT_MIN)
+				return DTERR_FIELD_OVERFLOW;
+			itm->tm_sec = itm->tm_min;
+			itm->tm_min = (int) itm->tm_hour;
+			itm->tm_hour = 0;
 		}
 	}
 	else if (*cp == '.')
 	{
 		/* always assume mm:ss.sss is MINUTE TO SECOND */
-		dterr = ParseFractionalSecond(cp, fsec);
+		dterr = ParseFractionalSecond(cp, &fsec);
 		if (dterr)
 			return dterr;
-		tm->tm_sec = tm->tm_min;
-		tm->tm_min = tm->tm_hour;
-		tm->tm_hour = 0;
+		if (itm->tm_hour > INT_MAX || itm->tm_hour < INT_MIN)
+			return DTERR_FIELD_OVERFLOW;
+		itm->tm_sec = itm->tm_min;
+		itm->tm_min = (int) itm->tm_hour;
+		itm->tm_hour = 0;
 	}
 	else if (*cp == ':')
 	{
 		errno = 0;
-		tm->tm_sec = strtoint(cp + 1, &cp, 10);
+		itm->tm_sec = strtoint(cp + 1, &cp, 10);
 		if (errno == ERANGE)
 			return DTERR_FIELD_OVERFLOW;
-		if (*cp == '\0')
-			*fsec = 0;
-		else if (*cp == '.')
+		if (*cp == '.')
 		{
-			dterr = ParseFractionalSecond(cp, fsec);
+			dterr = ParseFractionalSecond(cp, &fsec);
 			if (dterr)
 				return dterr;
 		}
-		else
+		else if (*cp != '\0')
 			return DTERR_BAD_FORMAT;
 	}
 	else
 		return DTERR_BAD_FORMAT;
 
-	/* do a sanity check */
-	if (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_min > MINS_PER_HOUR - 1 ||
-		tm->tm_sec < 0 || tm->tm_sec > SECS_PER_MINUTE ||
-		*fsec < INT64CONST(0) ||
-		*fsec > USECS_PER_SEC)
+	/* do a sanity check; but caller must check the range of tm_hour */
+	if (itm->tm_hour < 0 ||
+		itm->tm_min < 0 || itm->tm_min > MINS_PER_HOUR - 1 ||
+		itm->tm_sec < 0 || itm->tm_sec > SECS_PER_MINUTE ||
+		fsec < 0 || fsec > USECS_PER_SEC)
+		return DTERR_FIELD_OVERFLOW;
+
+	itm->tm_usec = (int) fsec;
+
+	return 0;
+}
+
+/* DecodeTime()
+ * Decode time string which includes delimiters.
+ * Return 0 if okay, a DTERR code if not.
+ *
+ * This version is used for timestamps.  The results are returned into
+ * the tm_hour/tm_min/tm_sec fields of *tm, and microseconds into *fsec.
+ */
+static int
+DecodeTime(char *str, int fmask, int range,
+		   int *tmask, struct pg_tm *tm, fsec_t *fsec)
+{
+	struct pg_itm itm;
+	int			dterr;
+
+	dterr = DecodeTimeCommon(str, fmask, range,
+							 tmask, &itm);
+	if (dterr)
+		return dterr;
+
+	if (itm.tm_hour > INT_MAX)
+		return DTERR_FIELD_OVERFLOW;
+	tm->tm_hour = (int) itm.tm_hour;
+	tm->tm_min = itm.tm_min;
+	tm->tm_sec = itm.tm_sec;
+	*fsec = itm.tm_usec;
+
+	return 0;
+}
+
+/* DecodeTimeForInterval()
+ * Decode time string which includes delimiters.
+ * Return 0 if okay, a DTERR code if not.
+ *
+ * This version is used for intervals.  The results are returned into
+ * itm_in->tm_usec.
+ */
+static int
+DecodeTimeForInterval(char *str, int fmask, int range,
+					  int *tmask, struct pg_itm_in *itm_in)
+{
+	struct pg_itm itm;
+	int			dterr;
+
+	dterr = DecodeTimeCommon(str, fmask, range,
+							 tmask, &itm);
+	if (dterr)
+		return dterr;
+
+	itm_in->tm_usec = itm.tm_usec;
+	if (!int64_multiply_add(itm.tm_hour, USECS_PER_HOUR, &itm_in->tm_usec) ||
+		!int64_multiply_add(itm.tm_min, USECS_PER_MINUTE, &itm_in->tm_usec) ||
+		!int64_multiply_add(itm.tm_sec, USECS_PER_SEC, &itm_in->tm_usec))
 		return DTERR_FIELD_OVERFLOW;
 
 	return 0;
@@ -3064,27 +3267,24 @@ DecodeSpecial(int field, char *lowtoken, int *val)
 }
 
 
-/* ClearPgTm
+/* ClearPgItmIn
  *
- * Zero out a pg_tm and associated fsec_t
+ * Zero out a pg_itm_in
  */
 static inline void
-ClearPgTm(struct pg_tm *tm, fsec_t *fsec)
+ClearPgItmIn(struct pg_itm_in *itm_in)
 {
-	tm->tm_year = 0;
-	tm->tm_mon = 0;
-	tm->tm_mday = 0;
-	tm->tm_hour = 0;
-	tm->tm_min = 0;
-	tm->tm_sec = 0;
-	*fsec = 0;
+	itm_in->tm_usec = 0;
+	itm_in->tm_mday = 0;
+	itm_in->tm_mon = 0;
+	itm_in->tm_year = 0;
 }
 
 
 /* DecodeInterval()
  * Interpret previously parsed fields for general time interval.
  * Returns 0 if successful, DTERR code if bogus input detected.
- * dtype, tm, fsec are output parameters.
+ * dtype and itm_in are output parameters.
  *
  * Allow "date" field DTK_DATE since this could be just
  *	an unsigned floating point number. - thomas 1997-11-16
@@ -3094,21 +3294,22 @@ ClearPgTm(struct pg_tm *tm, fsec_t *fsec)
  */
 int
 DecodeInterval(char **field, int *ftype, int nf, int range,
-			   int *dtype, struct pg_tm *tm, fsec_t *fsec)
+			   int *dtype, struct pg_itm_in *itm_in)
 {
 	bool		is_before = false;
 	char	   *cp;
 	int			fmask = 0,
 				tmask,
-				type;
+				type,
+				uval;
 	int			i;
 	int			dterr;
-	int			val;
+	int64		val;
 	double		fval;
 
 	*dtype = DTK_DELTA;
 	type = IGNORE_DTF;
-	ClearPgTm(tm, fsec);
+	ClearPgItmIn(itm_in);
 
 	/* read through list backwards to pick up units before values */
 	for (i = nf - 1; i >= 0; i--)
@@ -3116,8 +3317,8 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 		switch (ftype[i])
 		{
 			case DTK_TIME:
-				dterr = DecodeTime(field[i], fmask, range,
-								   &tmask, tm, fsec);
+				dterr = DecodeTimeForInterval(field[i], fmask, range,
+											  &tmask, itm_in);
 				if (dterr)
 					return dterr;
 				type = DTK_DAY;
@@ -3137,16 +3338,15 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 				 * like DTK_TIME case above, plus handling the sign.
 				 */
 				if (strchr(field[i] + 1, ':') != NULL &&
-					DecodeTime(field[i] + 1, fmask, range,
-							   &tmask, tm, fsec) == 0)
+					DecodeTimeForInterval(field[i] + 1, fmask, range,
+										  &tmask, itm_in) == 0)
 				{
 					if (*field[i] == '-')
 					{
-						/* flip the sign on all fields */
-						tm->tm_hour = -tm->tm_hour;
-						tm->tm_min = -tm->tm_min;
-						tm->tm_sec = -tm->tm_sec;
-						*fsec = -(*fsec);
+						/* flip the sign on time field */
+						if (itm_in->tm_usec == PG_INT64_MIN)
+							return DTERR_FIELD_OVERFLOW;
+						itm_in->tm_usec = -itm_in->tm_usec;
 					}
 
 					/*
@@ -3204,7 +3404,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 				}
 
 				errno = 0;
-				val = strtoint(field[i], &cp, 10);
+				val = strtoi64(field[i], &cp, 10);
 				if (errno == ERANGE)
 					return DTERR_FIELD_OVERFLOW;
 
@@ -3221,10 +3421,10 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 					type = DTK_MONTH;
 					if (*field[i] == '-')
 						val2 = -val2;
-					if (((double) val * MONTHS_PER_YEAR + val2) > INT_MAX ||
-						((double) val * MONTHS_PER_YEAR + val2) < INT_MIN)
+					if (pg_mul_s64_overflow(val, MONTHS_PER_YEAR, &val))
+						return DTERR_FIELD_OVERFLOW;
+					if (pg_add_s64_overflow(val, val2, &val))
 						return DTERR_FIELD_OVERFLOW;
-					val = val * MONTHS_PER_YEAR + val2;
 					fval = 0;
 				}
 				else if (*cp == '.')
@@ -3247,21 +3447,20 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 				switch (type)
 				{
 					case DTK_MICROSEC:
-						*fsec += rint(val + fval);
+						if (!AdjustMicroseconds(val, fval, 1, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MICROSECOND);
 						break;
 
 					case DTK_MILLISEC:
-						/* avoid overflowing the fsec field */
-						tm->tm_sec += val / 1000;
-						val -= (val / 1000) * 1000;
-						*fsec += rint((val + fval) * 1000);
+						if (!AdjustMicroseconds(val, fval, 1000, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MILLISECOND);
 						break;
 
 					case DTK_SECOND:
-						tm->tm_sec += val;
-						*fsec += rint(fval * 1000000);
+						if (!AdjustMicroseconds(val, fval, USECS_PER_SEC, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 
 						/*
 						 * If any subseconds were specified, consider this
@@ -3274,57 +3473,64 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_MINUTE:
-						tm->tm_min += val;
-						AdjustFractSeconds(fval, tm, fsec, SECS_PER_MINUTE);
+						if (!AdjustMicroseconds(val, fval, USECS_PER_MINUTE, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MINUTE);
 						break;
 
 					case DTK_HOUR:
-						tm->tm_hour += val;
-						AdjustFractSeconds(fval, tm, fsec, SECS_PER_HOUR);
+						if (!AdjustMicroseconds(val, fval, USECS_PER_HOUR, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(HOUR);
 						type = DTK_DAY; /* set for next field */
 						break;
 
 					case DTK_DAY:
-						tm->tm_mday += val;
-						AdjustFractSeconds(fval, tm, fsec, SECS_PER_DAY);
+						if (!AdjustDays(val, 1, itm_in) ||
+							!AdjustFractMicroseconds(fval, USECS_PER_DAY, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(DAY);
 						break;
 
 					case DTK_WEEK:
-						tm->tm_mday += val * 7;
-						AdjustFractDays(fval, tm, fsec, 7);
+						if (!AdjustDays(val, 7, itm_in) ||
+							!AdjustFractDays(fval, 7, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(WEEK);
 						break;
 
 					case DTK_MONTH:
-						tm->tm_mon += val;
-						AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+						if (!AdjustMonths(val, itm_in) ||
+							!AdjustFractDays(fval, DAYS_PER_MONTH, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MONTH);
 						break;
 
 					case DTK_YEAR:
-						tm->tm_year += val;
-						tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
+						if (!AdjustYears(val, 1, itm_in) ||
+							!AdjustFractYears(fval, 1, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(YEAR);
 						break;
 
 					case DTK_DECADE:
-						tm->tm_year += val * 10;
-						tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 10);
+						if (!AdjustYears(val, 10, itm_in) ||
+							!AdjustFractYears(fval, 10, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(DECADE);
 						break;
 
 					case DTK_CENTURY:
-						tm->tm_year += val * 100;
-						tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 100);
+						if (!AdjustYears(val, 100, itm_in) ||
+							!AdjustFractYears(fval, 100, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(CENTURY);
 						break;
 
 					case DTK_MILLENNIUM:
-						tm->tm_year += val * 1000;
-						tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 1000);
+						if (!AdjustYears(val, 1000, itm_in) ||
+							!AdjustFractYears(fval, 1000, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MILLENNIUM);
 						break;
 
@@ -3335,7 +3541,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 
 			case DTK_STRING:
 			case DTK_SPECIAL:
-				type = DecodeUnits(i, field[i], &val);
+				type = DecodeUnits(i, field[i], &uval);
 				if (type == IGNORE_DTF)
 					continue;
 
@@ -3343,17 +3549,17 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 				switch (type)
 				{
 					case UNITS:
-						type = val;
+						type = uval;
 						break;
 
 					case AGO:
 						is_before = true;
-						type = val;
+						type = uval;
 						break;
 
 					case RESERV:
 						tmask = (DTK_DATE_M | DTK_TIME_M);
-						*dtype = val;
+						*dtype = uval;
 						break;
 
 					default:
@@ -3374,16 +3580,6 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 	if (fmask == 0)
 		return DTERR_BAD_FORMAT;
 
-	/* ensure fractional seconds are fractional */
-	if (*fsec != 0)
-	{
-		int			sec;
-
-		sec = *fsec / USECS_PER_SEC;
-		*fsec -= sec * USECS_PER_SEC;
-		tm->tm_sec += sec;
-	}
-
 	/*----------
 	 * The SQL standard defines the interval literal
 	 *	 '-1 1:00:00'
@@ -3420,33 +3616,30 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 			 * Rather than re-determining which field was field[0], just force
 			 * 'em all negative.
 			 */
-			if (*fsec > 0)
-				*fsec = -(*fsec);
-			if (tm->tm_sec > 0)
-				tm->tm_sec = -tm->tm_sec;
-			if (tm->tm_min > 0)
-				tm->tm_min = -tm->tm_min;
-			if (tm->tm_hour > 0)
-				tm->tm_hour = -tm->tm_hour;
-			if (tm->tm_mday > 0)
-				tm->tm_mday = -tm->tm_mday;
-			if (tm->tm_mon > 0)
-				tm->tm_mon = -tm->tm_mon;
-			if (tm->tm_year > 0)
-				tm->tm_year = -tm->tm_year;
+			if (itm_in->tm_usec > 0)
+				itm_in->tm_usec = -itm_in->tm_usec;
+			if (itm_in->tm_mday > 0)
+				itm_in->tm_mday = -itm_in->tm_mday;
+			if (itm_in->tm_mon > 0)
+				itm_in->tm_mon = -itm_in->tm_mon;
+			if (itm_in->tm_year > 0)
+				itm_in->tm_year = -itm_in->tm_year;
 		}
 	}
 
 	/* finally, AGO negates everything */
 	if (is_before)
 	{
-		*fsec = -(*fsec);
-		tm->tm_sec = -tm->tm_sec;
-		tm->tm_min = -tm->tm_min;
-		tm->tm_hour = -tm->tm_hour;
-		tm->tm_mday = -tm->tm_mday;
-		tm->tm_mon = -tm->tm_mon;
-		tm->tm_year = -tm->tm_year;
+		if (itm_in->tm_usec == PG_INT64_MIN ||
+			itm_in->tm_mday == INT_MIN ||
+			itm_in->tm_mon == INT_MIN ||
+			itm_in->tm_year == INT_MIN)
+			return DTERR_FIELD_OVERFLOW;
+
+		itm_in->tm_usec = -itm_in->tm_usec;
+		itm_in->tm_mday = -itm_in->tm_mday;
+		itm_in->tm_mon = -itm_in->tm_mon;
+		itm_in->tm_year = -itm_in->tm_year;
 	}
 
 	return 0;
@@ -3460,26 +3653,35 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
  * Returns 0 or DTERR code.
  */
 static int
-ParseISO8601Number(char *str, char **endptr, int *ipart, double *fpart)
+ParseISO8601Number(char *str, char **endptr, int64 *ipart, double *fpart)
 {
-	double		val;
+	int			sign = 1;
 
-	if (!(isdigit((unsigned char) *str) || *str == '-' || *str == '.'))
-		return DTERR_BAD_FORMAT;
+	*ipart = 0;
+	*fpart = 0.0;
+
+	/* Parse sign if there is any */
+	if (*str == '-')
+	{
+		sign = -1;
+		str++;
+	}
+
+	*endptr = str;
 	errno = 0;
-	val = strtod(str, endptr);
-	/* did we not see anything that looks like a double? */
+
+	/* Parse int64 part if there is any */
+	if (isdigit((unsigned char) **endptr))
+		*ipart = strtoi64(*endptr, endptr, 10) * sign;
+
+	/* Parse fractional part if there is any */
+	if (**endptr == '.')
+		*fpart = strtod(*endptr, endptr) * sign;
+
+	/* did we not see anything that looks like a number? */
 	if (*endptr == str || errno != 0)
 		return DTERR_BAD_FORMAT;
-	/* watch out for overflow */
-	if (val < INT_MIN || val > INT_MAX)
-		return DTERR_FIELD_OVERFLOW;
-	/* be very sure we truncate towards zero (cf dtrunc()) */
-	if (val >= 0)
-		*ipart = (int) floor(val);
-	else
-		*ipart = (int) -floor(-val);
-	*fpart = val - *ipart;
+
 	return 0;
 }
 
@@ -3508,7 +3710,7 @@ ISO8601IntegerWidth(char *fieldstart)
  * Returns 0 if successful, DTERR code if bogus input detected.
  * Note: error code should be DTERR_BAD_FORMAT if input doesn't look like
  * ISO8601, otherwise this could cause unexpected error messages.
- * dtype, tm, fsec are output parameters.
+ * dtype and itm_in are output parameters.
  *
  *	A couple exceptions from the spec:
  *	 - a week field ('W') may coexist with other units
@@ -3516,13 +3718,13 @@ ISO8601IntegerWidth(char *fieldstart)
  */
 int
 DecodeISO8601Interval(char *str,
-					  int *dtype, struct pg_tm *tm, fsec_t *fsec)
+					  int *dtype, struct pg_itm_in *itm_in)
 {
 	bool		datepart = true;
 	bool		havefield = false;
 
 	*dtype = DTK_DELTA;
-	ClearPgTm(tm, fsec);
+	ClearPgItmIn(itm_in);
 
 	if (strlen(str) < 2 || str[0] != 'P')
 		return DTERR_BAD_FORMAT;
@@ -3531,7 +3733,7 @@ DecodeISO8601Interval(char *str,
 	while (*str)
 	{
 		char	   *fieldstart;
-		int			val;
+		int64		val;
 		double		fval;
 		char		unit;
 		int			dterr;
@@ -3560,29 +3762,34 @@ DecodeISO8601Interval(char *str,
 			switch (unit)		/* before T: Y M W D */
 			{
 				case 'Y':
-					tm->tm_year += val;
-					tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
+					if (!AdjustYears(val, 1, itm_in) ||
+						!AdjustFractYears(fval, 1, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					break;
 				case 'M':
-					tm->tm_mon += val;
-					AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+					if (!AdjustMonths(val, itm_in) ||
+						!AdjustFractDays(fval, DAYS_PER_MONTH, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					break;
 				case 'W':
-					tm->tm_mday += val * 7;
-					AdjustFractDays(fval, tm, fsec, 7);
+					if (!AdjustDays(val, 7, itm_in) ||
+						!AdjustFractDays(fval, 7, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					break;
 				case 'D':
-					tm->tm_mday += val;
-					AdjustFractSeconds(fval, tm, fsec, SECS_PER_DAY);
+					if (!AdjustDays(val, 1, itm_in) ||
+						!AdjustFractMicroseconds(fval, USECS_PER_DAY, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					break;
 				case 'T':		/* ISO 8601 4.4.3.3 Alternative Format / Basic */
 				case '\0':
 					if (ISO8601IntegerWidth(fieldstart) == 8 && !havefield)
 					{
-						tm->tm_year += val / 10000;
-						tm->tm_mon += (val / 100) % 100;
-						tm->tm_mday += val % 100;
-						AdjustFractSeconds(fval, tm, fsec, SECS_PER_DAY);
+						if (!AdjustYears(val / 10000, 1, itm_in) ||
+							!AdjustMonths((val / 100) % 100, itm_in) ||
+							!AdjustDays(val % 100, 1, itm_in) ||
+							!AdjustFractMicroseconds(fval, USECS_PER_DAY, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						if (unit == '\0')
 							return 0;
 						datepart = false;
@@ -3596,8 +3803,9 @@ DecodeISO8601Interval(char *str,
 					if (havefield)
 						return DTERR_BAD_FORMAT;
 
-					tm->tm_year += val;
-					tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
+					if (!AdjustYears(val, 1, itm_in) ||
+						!AdjustFractYears(fval, 1, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					if (unit == '\0')
 						return 0;
 					if (unit == 'T')
@@ -3610,8 +3818,9 @@ DecodeISO8601Interval(char *str,
 					dterr = ParseISO8601Number(str, &str, &val, &fval);
 					if (dterr)
 						return dterr;
-					tm->tm_mon += val;
-					AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+					if (!AdjustMonths(val, itm_in) ||
+						!AdjustFractDays(fval, DAYS_PER_MONTH, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					if (*str == '\0')
 						return 0;
 					if (*str == 'T')
@@ -3627,8 +3836,9 @@ DecodeISO8601Interval(char *str,
 					dterr = ParseISO8601Number(str, &str, &val, &fval);
 					if (dterr)
 						return dterr;
-					tm->tm_mday += val;
-					AdjustFractSeconds(fval, tm, fsec, SECS_PER_DAY);
+					if (!AdjustDays(val, 1, itm_in) ||
+						!AdjustFractMicroseconds(fval, USECS_PER_DAY, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					if (*str == '\0')
 						return 0;
 					if (*str == 'T')
@@ -3648,24 +3858,25 @@ DecodeISO8601Interval(char *str,
 			switch (unit)		/* after T: H M S */
 			{
 				case 'H':
-					tm->tm_hour += val;
-					AdjustFractSeconds(fval, tm, fsec, SECS_PER_HOUR);
+					if (!AdjustMicroseconds(val, fval, USECS_PER_HOUR, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					break;
 				case 'M':
-					tm->tm_min += val;
-					AdjustFractSeconds(fval, tm, fsec, SECS_PER_MINUTE);
+					if (!AdjustMicroseconds(val, fval, USECS_PER_MINUTE, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					break;
 				case 'S':
-					tm->tm_sec += val;
-					AdjustFractSeconds(fval, tm, fsec, 1);
+					if (!AdjustMicroseconds(val, fval, USECS_PER_SEC, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					break;
 				case '\0':		/* ISO 8601 4.4.3.3 Alternative Format */
 					if (ISO8601IntegerWidth(fieldstart) == 6 && !havefield)
 					{
-						tm->tm_hour += val / 10000;
-						tm->tm_min += (val / 100) % 100;
-						tm->tm_sec += val % 100;
-						AdjustFractSeconds(fval, tm, fsec, 1);
+						if (!AdjustMicroseconds(val / 10000, 0, USECS_PER_HOUR, itm_in) ||
+							!AdjustMicroseconds((val / 100) % 100, 0, USECS_PER_MINUTE, itm_in) ||
+							!AdjustMicroseconds(val % 100, 0, USECS_PER_SEC, itm_in) ||
+							!AdjustFractMicroseconds(fval, 1, itm_in))
+							return DTERR_FIELD_OVERFLOW;
 						return 0;
 					}
 					/* Else fall through to extended alternative format */
@@ -3675,16 +3886,16 @@ DecodeISO8601Interval(char *str,
 					if (havefield)
 						return DTERR_BAD_FORMAT;
 
-					tm->tm_hour += val;
-					AdjustFractSeconds(fval, tm, fsec, SECS_PER_HOUR);
+					if (!AdjustMicroseconds(val, fval, USECS_PER_HOUR, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					if (unit == '\0')
 						return 0;
 
 					dterr = ParseISO8601Number(str, &str, &val, &fval);
 					if (dterr)
 						return dterr;
-					tm->tm_min += val;
-					AdjustFractSeconds(fval, tm, fsec, SECS_PER_MINUTE);
+					if (!AdjustMicroseconds(val, fval, USECS_PER_MINUTE, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					if (*str == '\0')
 						return 0;
 					if (*str != ':')
@@ -3694,8 +3905,8 @@ DecodeISO8601Interval(char *str,
 					dterr = ParseISO8601Number(str, &str, &val, &fval);
 					if (dterr)
 						return dterr;
-					tm->tm_sec += val;
-					AdjustFractSeconds(fval, tm, fsec, 1);
+					if (!AdjustMicroseconds(val, fval, USECS_PER_SEC, itm_in))
+						return DTERR_FIELD_OVERFLOW;
 					if (*str == '\0')
 						return 0;
 					return DTERR_BAD_FORMAT;
@@ -4166,25 +4377,25 @@ EncodeDateTime(struct pg_tm *tm, fsec_t fsec, bool print_tz, int tz, const char
 
 /* Append an ISO-8601-style interval field, but only if value isn't zero */
 static char *
-AddISO8601IntPart(char *cp, int value, char units)
+AddISO8601IntPart(char *cp, int64 value, char units)
 {
 	if (value == 0)
 		return cp;
-	sprintf(cp, "%d%c", value, units);
+	sprintf(cp, "%lld%c", (long long) value, units);
 	return cp + strlen(cp);
 }
 
 /* Append a postgres-style interval field, but only if value isn't zero */
 static char *
-AddPostgresIntPart(char *cp, int value, const char *units,
+AddPostgresIntPart(char *cp, int64 value, const char *units,
 				   bool *is_zero, bool *is_before)
 {
 	if (value == 0)
 		return cp;
-	sprintf(cp, "%s%s%d %s%s",
+	sprintf(cp, "%s%s%lld %s%s",
 			(!*is_zero) ? " " : "",
 			(*is_before && value > 0) ? "+" : "",
-			value,
+			(long long) value,
 			units,
 			(value != 1) ? "s" : "");
 
@@ -4199,7 +4410,7 @@ AddPostgresIntPart(char *cp, int value, const char *units,
 
 /* Append a verbose-style interval field, but only if value isn't zero */
 static char *
-AddVerboseIntPart(char *cp, int value, const char *units,
+AddVerboseIntPart(char *cp, int64 value, const char *units,
 				  bool *is_zero, bool *is_before)
 {
 	if (value == 0)
@@ -4208,11 +4419,11 @@ AddVerboseIntPart(char *cp, int value, const char *units,
 	if (*is_zero)
 	{
 		*is_before = (value < 0);
-		value = abs(value);
+		value = Abs(value);
 	}
 	else if (*is_before)
 		value = -value;
-	sprintf(cp, " %d %s%s", value, units, (value == 1) ? "" : "s");
+	sprintf(cp, " %lld %s%s", (long long) value, units, (value == 1) ? "" : "s");
 	*is_zero = false;
 	return cp + strlen(cp);
 }
@@ -4238,15 +4449,16 @@ AddVerboseIntPart(char *cp, int value, const char *units,
  * "day-time literal"s (that look like ('4 5:6:7')
  */
 void
-EncodeInterval(struct pg_tm *tm, fsec_t fsec, int style, char *str)
+EncodeInterval(struct pg_itm *itm, int style, char *str)
 {
 	char	   *cp = str;
-	int			year = tm->tm_year;
-	int			mon = tm->tm_mon;
-	int			mday = tm->tm_mday;
-	int			hour = tm->tm_hour;
-	int			min = tm->tm_min;
-	int			sec = tm->tm_sec;
+	int			year = itm->tm_year;
+	int			mon = itm->tm_mon;
+	int64		mday = itm->tm_mday;	/* tm_mday could be INT_MIN */
+	int64		hour = itm->tm_hour;
+	int			min = itm->tm_min;
+	int			sec = itm->tm_sec;
+	int			fsec = itm->tm_usec;
 	bool		is_before = false;
 	bool		is_zero = true;
 
@@ -4306,10 +4518,10 @@ EncodeInterval(struct pg_tm *tm, fsec_t fsec, int style, char *str)
 					char		sec_sign = (hour < 0 || min < 0 ||
 											sec < 0 || fsec < 0) ? '-' : '+';
 
-					sprintf(cp, "%c%d-%d %c%d %c%d:%02d:",
+					sprintf(cp, "%c%d-%d %c%lld %c%lld:%02d:",
 							year_sign, abs(year), abs(mon),
-							day_sign, abs(mday),
-							sec_sign, abs(hour), abs(min));
+							day_sign, (long long) Abs(mday),
+							sec_sign, (long long) Abs(hour), abs(min));
 					cp += strlen(cp);
 					cp = AppendSeconds(cp, sec, fsec, MAX_INTERVAL_PRECISION, true);
 					*cp = '\0';
@@ -4320,14 +4532,15 @@ EncodeInterval(struct pg_tm *tm, fsec_t fsec, int style, char *str)
 				}
 				else if (has_day)
 				{
-					sprintf(cp, "%d %d:%02d:", mday, hour, min);
+					sprintf(cp, "%lld %lld:%02d:",
+							(long long) mday, (long long) hour, min);
 					cp += strlen(cp);
 					cp = AppendSeconds(cp, sec, fsec, MAX_INTERVAL_PRECISION, true);
 					*cp = '\0';
 				}
 				else
 				{
-					sprintf(cp, "%d:%02d:", hour, min);
+					sprintf(cp, "%lld:%02d:", (long long) hour, min);
 					cp += strlen(cp);
 					cp = AppendSeconds(cp, sec, fsec, MAX_INTERVAL_PRECISION, true);
 					*cp = '\0';
@@ -4377,10 +4590,10 @@ EncodeInterval(struct pg_tm *tm, fsec_t fsec, int style, char *str)
 			{
 				bool		minus = (hour < 0 || min < 0 || sec < 0 || fsec < 0);
 
-				sprintf(cp, "%s%s%02d:%02d:",
+				sprintf(cp, "%s%s%02lld:%02d:",
 						is_zero ? "" : " ",
 						(minus ? "-" : (is_before ? "+" : "")),
-						abs(hour), abs(min));
+						(long long) Abs(hour), abs(min));
 				cp += strlen(cp);
 				cp = AppendSeconds(cp, sec, fsec, MAX_INTERVAL_PRECISION, true);
 				*cp = '\0';
@@ -4668,7 +4881,7 @@ pg_timezone_abbrevs(PG_FUNCTION_ARGS)
 	int			gmtoffset;
 	bool		is_dst;
 	unsigned char *p;
-	struct pg_tm tm;
+	struct pg_itm_in itm_in;
 	Interval   *resInterval;
 
 	/* stuff done only on the first call of the function */
@@ -4761,11 +4974,11 @@ pg_timezone_abbrevs(PG_FUNCTION_ARGS)
 
 	values[0] = CStringGetTextDatum(buffer);
 
-	/* Convert offset (in seconds) to an interval */
-	MemSet(&tm, 0, sizeof(struct pg_tm));
-	tm.tm_sec = gmtoffset;
+	/* Convert offset (in seconds) to an interval; can't overflow */
+	MemSet(&itm_in, 0, sizeof(struct pg_itm_in));
+	itm_in.tm_usec = (int64) gmtoffset * USECS_PER_SEC;
 	resInterval = (Interval *) palloc(sizeof(Interval));
-	tm2interval(&tm, 0, resInterval);
+	(void) itmin2interval(&itm_in, resInterval);
 	values[1] = IntervalPGetDatum(resInterval);
 
 	values[2] = BoolGetDatum(is_dst);
@@ -4795,7 +5008,7 @@ pg_timezone_names(PG_FUNCTION_ARGS)
 	fsec_t		fsec;
 	const char *tzn;
 	Interval   *resInterval;
-	struct pg_tm itm;
+	struct pg_itm_in itm_in;
 
 	SetSingleFuncCall(fcinfo, 0);
 
@@ -4831,10 +5044,11 @@ pg_timezone_names(PG_FUNCTION_ARGS)
 		values[0] = CStringGetTextDatum(pg_get_timezone_name(tz));
 		values[1] = CStringGetTextDatum(tzn ? tzn : "");
 
-		MemSet(&itm, 0, sizeof(struct pg_tm));
-		itm.tm_sec = -tzoff;
+		/* Convert tzoff to an interval; can't overflow */
+		MemSet(&itm_in, 0, sizeof(struct pg_itm_in));
+		itm_in.tm_usec = (int64) -tzoff * USECS_PER_SEC;
 		resInterval = (Interval *) palloc(sizeof(Interval));
-		tm2interval(&itm, 0, resInterval);
+		(void) itmin2interval(&itm_in, resInterval);
 		values[2] = IntervalPGetDatum(resInterval);
 
 		values[3] = BoolGetDatum(tm.tm_isdst > 0);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index ac74333be5..843b07d7d2 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -491,11 +491,28 @@ typedef struct
 
 /* ----------
  * Datetime to char conversion
+ *
+ * To support intervals as well as timestamps, we use a custom "tm" struct
+ * that is almost like struct pg_tm, but has a 64-bit tm_hour field.
+ * We omit the tm_isdst and tm_zone fields, which are not used here.
  * ----------
  */
+struct fmt_tm
+{
+	int			tm_sec;
+	int			tm_min;
+	int64		tm_hour;
+	int			tm_mday;
+	int			tm_mon;
+	int			tm_year;
+	int			tm_wday;
+	int			tm_yday;
+	long int	tm_gmtoff;
+};
+
 typedef struct TmToChar
 {
-	struct pg_tm tm;			/* classic 'tm' struct */
+	struct fmt_tm tm;			/* almost the classic 'tm' struct */
 	fsec_t		fsec;			/* fractional seconds */
 	const char *tzn;			/* timezone */
 } TmToChar;
@@ -504,12 +521,25 @@ typedef struct TmToChar
 #define tmtcTzn(_X) ((_X)->tzn)
 #define tmtcFsec(_X)	((_X)->fsec)
 
+/* Note: this is used to copy pg_tm to fmt_tm, so not quite a bitwise copy */
+#define COPY_tm(_DST, _SRC) \
+do {	\
+	(_DST)->tm_sec = (_SRC)->tm_sec; \
+	(_DST)->tm_min = (_SRC)->tm_min; \
+	(_DST)->tm_hour = (_SRC)->tm_hour; \
+	(_DST)->tm_mday = (_SRC)->tm_mday; \
+	(_DST)->tm_mon = (_SRC)->tm_mon; \
+	(_DST)->tm_year = (_SRC)->tm_year; \
+	(_DST)->tm_wday = (_SRC)->tm_wday; \
+	(_DST)->tm_yday = (_SRC)->tm_yday; \
+	(_DST)->tm_gmtoff = (_SRC)->tm_gmtoff; \
+} while(0)
+
+/* Caution: this is used to zero both pg_tm and fmt_tm structs */
 #define ZERO_tm(_X) \
 do {	\
-	(_X)->tm_sec  = (_X)->tm_year = (_X)->tm_min = (_X)->tm_wday = \
-	(_X)->tm_hour = (_X)->tm_yday = (_X)->tm_isdst = 0; \
-	(_X)->tm_mday = (_X)->tm_mon  = 1; \
-	(_X)->tm_zone = NULL; \
+	memset(_X, 0, sizeof(*(_X))); \
+	(_X)->tm_mday = (_X)->tm_mon = 1; \
 } while(0)
 
 #define ZERO_tmtc(_X) \
@@ -2649,7 +2679,7 @@ DCH_to_char(FormatNode *node, bool is_interval, TmToChar *in, char *out, Oid col
 {
 	FormatNode *n;
 	char	   *s;
-	struct pg_tm *tm = &in->tm;
+	struct fmt_tm *tm = &in->tm;
 	int			i;
 
 	/* cache localized days and months */
@@ -2698,16 +2728,17 @@ DCH_to_char(FormatNode *node, bool is_interval, TmToChar *in, char *out, Oid col
 				 * display time as shown on a 12-hour clock, even for
 				 * intervals
 				 */
-				sprintf(s, "%0*d", S_FM(n->suffix) ? 0 : (tm->tm_hour >= 0) ? 2 : 3,
-						tm->tm_hour % (HOURS_PER_DAY / 2) == 0 ? HOURS_PER_DAY / 2 :
-						tm->tm_hour % (HOURS_PER_DAY / 2));
+				sprintf(s, "%0*lld", S_FM(n->suffix) ? 0 : (tm->tm_hour >= 0) ? 2 : 3,
+						tm->tm_hour % (HOURS_PER_DAY / 2) == 0 ?
+						(long long) (HOURS_PER_DAY / 2) :
+						(long long) (tm->tm_hour % (HOURS_PER_DAY / 2)));
 				if (S_THth(n->suffix))
 					str_numth(s, s, S_TH_TYPE(n->suffix));
 				s += strlen(s);
 				break;
 			case DCH_HH24:
-				sprintf(s, "%0*d", S_FM(n->suffix) ? 0 : (tm->tm_hour >= 0) ? 2 : 3,
-						tm->tm_hour);
+				sprintf(s, "%0*lld", S_FM(n->suffix) ? 0 : (tm->tm_hour >= 0) ? 2 : 3,
+						(long long) tm->tm_hour);
 				if (S_THth(n->suffix))
 					str_numth(s, s, S_TH_TYPE(n->suffix));
 				s += strlen(s);
@@ -2755,9 +2786,10 @@ DCH_to_char(FormatNode *node, bool is_interval, TmToChar *in, char *out, Oid col
 				break;
 #undef DCH_to_char_fsec
 			case DCH_SSSS:
-				sprintf(s, "%d", tm->tm_hour * SECS_PER_HOUR +
-						tm->tm_min * SECS_PER_MINUTE +
-						tm->tm_sec);
+				sprintf(s, "%lld",
+						(long long) (tm->tm_hour * SECS_PER_HOUR +
+									 tm->tm_min * SECS_PER_MINUTE +
+									 tm->tm_sec));
 				if (S_THth(n->suffix))
 					str_numth(s, s, S_TH_TYPE(n->suffix));
 				s += strlen(s);
@@ -4088,7 +4120,8 @@ timestamp_to_char(PG_FUNCTION_ARGS)
 	text	   *fmt = PG_GETARG_TEXT_PP(1),
 			   *res;
 	TmToChar	tmtc;
-	struct pg_tm *tm;
+	struct pg_tm tt;
+	struct fmt_tm *tm;
 	int			thisdate;
 
 	if (VARSIZE_ANY_EXHDR(fmt) <= 0 || TIMESTAMP_NOT_FINITE(dt))
@@ -4097,10 +4130,11 @@ timestamp_to_char(PG_FUNCTION_ARGS)
 	ZERO_tmtc(&tmtc);
 	tm = tmtcTm(&tmtc);
 
-	if (timestamp2tm(dt, NULL, tm, &tmtcFsec(&tmtc), NULL, NULL) != 0)
+	if (timestamp2tm(dt, NULL, &tt, &tmtcFsec(&tmtc), NULL, NULL) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range")));
+	COPY_tm(tm, &tt);
 
 	thisdate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday);
 	tm->tm_wday = (thisdate + 1) % 7;
@@ -4120,7 +4154,8 @@ timestamptz_to_char(PG_FUNCTION_ARGS)
 			   *res;
 	TmToChar	tmtc;
 	int			tz;
-	struct pg_tm *tm;
+	struct pg_tm tt;
+	struct fmt_tm *tm;
 	int			thisdate;
 
 	if (VARSIZE_ANY_EXHDR(fmt) <= 0 || TIMESTAMP_NOT_FINITE(dt))
@@ -4129,10 +4164,11 @@ timestamptz_to_char(PG_FUNCTION_ARGS)
 	ZERO_tmtc(&tmtc);
 	tm = tmtcTm(&tmtc);
 
-	if (timestamp2tm(dt, &tz, tm, &tmtcFsec(&tmtc), &tmtcTzn(&tmtc), NULL) != 0)
+	if (timestamp2tm(dt, &tz, &tt, &tmtcFsec(&tmtc), &tmtcTzn(&tmtc), NULL) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("timestamp out of range")));
+	COPY_tm(tm, &tt);
 
 	thisdate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday);
 	tm->tm_wday = (thisdate + 1) % 7;
@@ -4156,7 +4192,9 @@ interval_to_char(PG_FUNCTION_ARGS)
 	text	   *fmt = PG_GETARG_TEXT_PP(1),
 			   *res;
 	TmToChar	tmtc;
-	struct pg_tm *tm;
+	struct fmt_tm *tm;
+	struct pg_itm tt,
+			   *itm = &tt;
 
 	if (VARSIZE_ANY_EXHDR(fmt) <= 0)
 		PG_RETURN_NULL();
@@ -4164,8 +4202,14 @@ interval_to_char(PG_FUNCTION_ARGS)
 	ZERO_tmtc(&tmtc);
 	tm = tmtcTm(&tmtc);
 
-	if (interval2tm(*it, tm, &tmtcFsec(&tmtc)) != 0)
-		PG_RETURN_NULL();
+	interval2itm(*it, itm);
+	tmtc.fsec = itm->tm_usec;
+	tm->tm_sec = itm->tm_sec;
+	tm->tm_min = itm->tm_min;
+	tm->tm_hour = itm->tm_hour;
+	tm->tm_mday = itm->tm_mday;
+	tm->tm_mon = itm->tm_mon;
+	tm->tm_year = itm->tm_year;
 
 	/* wday is meaningless, yday approximates the total span in days */
 	tm->tm_yday = (tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon) * DAYS_PER_MONTH + tm->tm_mday;
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index ae36ff3328..1f2ddfb73a 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -888,9 +888,8 @@ interval_in(PG_FUNCTION_ARGS)
 #endif
 	int32		typmod = PG_GETARG_INT32(2);
 	Interval   *result;
-	fsec_t		fsec;
-	struct pg_tm tt,
-			   *tm = &tt;
+	struct pg_itm_in tt,
+			   *itm_in = &tt;
 	int			dtype;
 	int			nf;
 	int			range;
@@ -899,13 +898,10 @@ interval_in(PG_FUNCTION_ARGS)
 	int			ftype[MAXDATEFIELDS];
 	char		workbuf[256];
 
-	tm->tm_year = 0;
-	tm->tm_mon = 0;
-	tm->tm_mday = 0;
-	tm->tm_hour = 0;
-	tm->tm_min = 0;
-	tm->tm_sec = 0;
-	fsec = 0;
+	itm_in->tm_year = 0;
+	itm_in->tm_mon = 0;
+	itm_in->tm_mday = 0;
+	itm_in->tm_usec = 0;
 
 	if (typmod >= 0)
 		range = INTERVAL_RANGE(typmod);
@@ -916,12 +912,12 @@ interval_in(PG_FUNCTION_ARGS)
 						  ftype, MAXDATEFIELDS, &nf);
 	if (dterr == 0)
 		dterr = DecodeInterval(field, ftype, nf, range,
-							   &dtype, tm, &fsec);
+							   &dtype, itm_in);
 
 	/* if those functions think it's a bad format, try ISO8601 style */
 	if (dterr == DTERR_BAD_FORMAT)
 		dterr = DecodeISO8601Interval(str,
-									  &dtype, tm, &fsec);
+									  &dtype, itm_in);
 
 	if (dterr != 0)
 	{
@@ -935,7 +931,7 @@ interval_in(PG_FUNCTION_ARGS)
 	switch (dtype)
 	{
 		case DTK_DELTA:
-			if (tm2interval(tm, fsec, result) != 0)
+			if (itmin2interval(itm_in, result) != 0)
 				ereport(ERROR,
 						(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 						 errmsg("interval out of range")));
@@ -959,15 +955,12 @@ interval_out(PG_FUNCTION_ARGS)
 {
 	Interval   *span = PG_GETARG_INTERVAL_P(0);
 	char	   *result;
-	struct pg_tm tt,
-			   *tm = &tt;
-	fsec_t		fsec;
+	struct pg_itm tt,
+			   *itm = &tt;
 	char		buf[MAXDATELEN + 1];
 
-	if (interval2tm(*span, tm, &fsec) != 0)
-		elog(ERROR, "could not convert interval to tm");
-
-	EncodeInterval(tm, fsec, IntervalStyle, buf);
+	interval2itm(*span, itm);
+	EncodeInterval(itm, IntervalStyle, buf);
 
 	result = pstrdup(buf);
 	PG_RETURN_CSTRING(result);
@@ -1959,50 +1952,77 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
 }
 
 
-/* interval2tm()
- * Convert an interval data type to a tm structure.
+/* interval2itm()
+ * Convert an Interval to a pg_itm structure.
+ * Note: overflow is not possible, because the pg_itm fields are
+ * wide enough for all possible conversion results.
  */
-int
-interval2tm(Interval span, struct pg_tm *tm, fsec_t *fsec)
+void
+interval2itm(Interval span, struct pg_itm *itm)
 {
 	TimeOffset	time;
 	TimeOffset	tfrac;
 
-	tm->tm_year = span.month / MONTHS_PER_YEAR;
-	tm->tm_mon = span.month % MONTHS_PER_YEAR;
-	tm->tm_mday = span.day;
+	itm->tm_year = span.month / MONTHS_PER_YEAR;
+	itm->tm_mon = span.month % MONTHS_PER_YEAR;
+	itm->tm_mday = span.day;
 	time = span.time;
 
 	tfrac = time / USECS_PER_HOUR;
 	time -= tfrac * USECS_PER_HOUR;
-	tm->tm_hour = tfrac;
-	if (!SAMESIGN(tm->tm_hour, tfrac))
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("interval out of range")));
+	itm->tm_hour = tfrac;
 	tfrac = time / USECS_PER_MINUTE;
 	time -= tfrac * USECS_PER_MINUTE;
-	tm->tm_min = tfrac;
+	itm->tm_min = (int) tfrac;
 	tfrac = time / USECS_PER_SEC;
-	*fsec = time - (tfrac * USECS_PER_SEC);
-	tm->tm_sec = tfrac;
+	time -= tfrac * USECS_PER_SEC;
+	itm->tm_sec = (int) tfrac;
+	itm->tm_usec = (int) time;
+}
 
+/* itm2interval()
+ * Convert a pg_itm structure to an Interval.
+ * Returns 0 if OK, -1 on overflow.
+ */
+int
+itm2interval(struct pg_itm *itm, Interval *span)
+{
+	int64		total_months = (int64) itm->tm_year * MONTHS_PER_YEAR + itm->tm_mon;
+
+	if (total_months > INT_MAX || total_months < INT_MIN)
+		return -1;
+	span->month = (int32) total_months;
+	span->day = itm->tm_mday;
+	if (pg_mul_s64_overflow(itm->tm_hour, USECS_PER_HOUR,
+							&span->time))
+		return -1;
+	/* tm_min, tm_sec are 32 bits, so intermediate products can't overflow */
+	if (pg_add_s64_overflow(span->time, itm->tm_min * USECS_PER_MINUTE,
+							&span->time))
+		return -1;
+	if (pg_add_s64_overflow(span->time, itm->tm_sec * USECS_PER_SEC,
+							&span->time))
+		return -1;
+	if (pg_add_s64_overflow(span->time, itm->tm_usec,
+							&span->time))
+		return -1;
 	return 0;
 }
 
+/* itmin2interval()
+ * Convert a pg_itm_in structure to an Interval.
+ * Returns 0 if OK, -1 on overflow.
+ */
 int
-tm2interval(struct pg_tm *tm, fsec_t fsec, Interval *span)
+itmin2interval(struct pg_itm_in *itm_in, Interval *span)
 {
-	double		total_months = (double) tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
+	int64		total_months = (int64) itm_in->tm_year * MONTHS_PER_YEAR + itm_in->tm_mon;
 
 	if (total_months > INT_MAX || total_months < INT_MIN)
 		return -1;
-	span->month = total_months;
-	span->day = tm->tm_mday;
-	span->time = (((((tm->tm_hour * INT64CONST(60)) +
-					 tm->tm_min) * INT64CONST(60)) +
-				   tm->tm_sec) * USECS_PER_SEC) + fsec;
-
+	span->month = (int32) total_months;
+	span->day = itm_in->tm_mday;
+	span->time = itm_in->tm_usec;
 	return 0;
 }
 
@@ -3601,10 +3621,9 @@ timestamp_age(PG_FUNCTION_ARGS)
 	Timestamp	dt1 = PG_GETARG_TIMESTAMP(0);
 	Timestamp	dt2 = PG_GETARG_TIMESTAMP(1);
 	Interval   *result;
-	fsec_t		fsec,
-				fsec1,
+	fsec_t		fsec1,
 				fsec2;
-	struct pg_tm tt,
+	struct pg_itm tt,
 			   *tm = &tt;
 	struct pg_tm tt1,
 			   *tm1 = &tt1;
@@ -3617,7 +3636,7 @@ timestamp_age(PG_FUNCTION_ARGS)
 		timestamp2tm(dt2, NULL, tm2, &fsec2, NULL, NULL) == 0)
 	{
 		/* form the symbolic difference */
-		fsec = fsec1 - fsec2;
+		tm->tm_usec = fsec1 - fsec2;
 		tm->tm_sec = tm1->tm_sec - tm2->tm_sec;
 		tm->tm_min = tm1->tm_min - tm2->tm_min;
 		tm->tm_hour = tm1->tm_hour - tm2->tm_hour;
@@ -3628,7 +3647,7 @@ timestamp_age(PG_FUNCTION_ARGS)
 		/* flip sign if necessary... */
 		if (dt1 < dt2)
 		{
-			fsec = -fsec;
+			tm->tm_usec = -tm->tm_usec;
 			tm->tm_sec = -tm->tm_sec;
 			tm->tm_min = -tm->tm_min;
 			tm->tm_hour = -tm->tm_hour;
@@ -3638,9 +3657,9 @@ timestamp_age(PG_FUNCTION_ARGS)
 		}
 
 		/* propagate any negative fields into the next higher field */
-		while (fsec < 0)
+		while (tm->tm_usec < 0)
 		{
-			fsec += USECS_PER_SEC;
+			tm->tm_usec += USECS_PER_SEC;
 			tm->tm_sec--;
 		}
 
@@ -3685,7 +3704,7 @@ timestamp_age(PG_FUNCTION_ARGS)
 		/* recover sign if necessary... */
 		if (dt1 < dt2)
 		{
-			fsec = -fsec;
+			tm->tm_usec = -tm->tm_usec;
 			tm->tm_sec = -tm->tm_sec;
 			tm->tm_min = -tm->tm_min;
 			tm->tm_hour = -tm->tm_hour;
@@ -3694,7 +3713,7 @@ timestamp_age(PG_FUNCTION_ARGS)
 			tm->tm_year = -tm->tm_year;
 		}
 
-		if (tm2interval(tm, fsec, result) != 0)
+		if (itm2interval(tm, result) != 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 					 errmsg("interval out of range")));
@@ -3720,10 +3739,9 @@ timestamptz_age(PG_FUNCTION_ARGS)
 	TimestampTz dt1 = PG_GETARG_TIMESTAMPTZ(0);
 	TimestampTz dt2 = PG_GETARG_TIMESTAMPTZ(1);
 	Interval   *result;
-	fsec_t		fsec,
-				fsec1,
+	fsec_t		fsec1,
 				fsec2;
-	struct pg_tm tt,
+	struct pg_itm tt,
 			   *tm = &tt;
 	struct pg_tm tt1,
 			   *tm1 = &tt1;
@@ -3738,7 +3756,7 @@ timestamptz_age(PG_FUNCTION_ARGS)
 		timestamp2tm(dt2, &tz2, tm2, &fsec2, NULL, NULL) == 0)
 	{
 		/* form the symbolic difference */
-		fsec = fsec1 - fsec2;
+		tm->tm_usec = fsec1 - fsec2;
 		tm->tm_sec = tm1->tm_sec - tm2->tm_sec;
 		tm->tm_min = tm1->tm_min - tm2->tm_min;
 		tm->tm_hour = tm1->tm_hour - tm2->tm_hour;
@@ -3749,7 +3767,7 @@ timestamptz_age(PG_FUNCTION_ARGS)
 		/* flip sign if necessary... */
 		if (dt1 < dt2)
 		{
-			fsec = -fsec;
+			tm->tm_usec = -tm->tm_usec;
 			tm->tm_sec = -tm->tm_sec;
 			tm->tm_min = -tm->tm_min;
 			tm->tm_hour = -tm->tm_hour;
@@ -3759,9 +3777,9 @@ timestamptz_age(PG_FUNCTION_ARGS)
 		}
 
 		/* propagate any negative fields into the next higher field */
-		while (fsec < 0)
+		while (tm->tm_usec < 0)
 		{
-			fsec += USECS_PER_SEC;
+			tm->tm_usec += USECS_PER_SEC;
 			tm->tm_sec--;
 		}
 
@@ -3810,7 +3828,7 @@ timestamptz_age(PG_FUNCTION_ARGS)
 		/* recover sign if necessary... */
 		if (dt1 < dt2)
 		{
-			fsec = -fsec;
+			tm->tm_usec = -tm->tm_usec;
 			tm->tm_sec = -tm->tm_sec;
 			tm->tm_min = -tm->tm_min;
 			tm->tm_hour = -tm->tm_hour;
@@ -3819,7 +3837,7 @@ timestamptz_age(PG_FUNCTION_ARGS)
 			tm->tm_year = -tm->tm_year;
 		}
 
-		if (tm2interval(tm, fsec, result) != 0)
+		if (itm2interval(tm, result) != 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 					 errmsg("interval out of range")));
@@ -4306,8 +4324,7 @@ interval_trunc(PG_FUNCTION_ARGS)
 	int			type,
 				val;
 	char	   *lowunits;
-	fsec_t		fsec;
-	struct pg_tm tt,
+	struct pg_itm tt,
 			   *tm = &tt;
 
 	result = (Interval *) palloc(sizeof(Interval));
@@ -4320,7 +4337,7 @@ interval_trunc(PG_FUNCTION_ARGS)
 
 	if (type == UNITS)
 	{
-		if (interval2tm(*interval, tm, &fsec) == 0)
+		interval2itm(*interval, tm);
 		{
 			switch (val)
 			{
@@ -4355,10 +4372,10 @@ interval_trunc(PG_FUNCTION_ARGS)
 					tm->tm_sec = 0;
 					/* FALL THRU */
 				case DTK_SECOND:
-					fsec = 0;
+					tm->tm_usec = 0;
 					break;
 				case DTK_MILLISEC:
-					fsec = (fsec / 1000) * 1000;
+					tm->tm_usec = (tm->tm_usec / 1000) * 1000;
 					break;
 				case DTK_MICROSEC:
 					break;
@@ -4371,13 +4388,11 @@ interval_trunc(PG_FUNCTION_ARGS)
 							 (val == DTK_WEEK) ? errdetail("Months usually have fractional weeks.") : 0));
 			}
 
-			if (tm2interval(tm, fsec, result) != 0)
+			if (itm2interval(tm, result) != 0)
 				ereport(ERROR,
 						(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 						 errmsg("interval out of range")));
 		}
-		else
-			elog(ERROR, "could not convert interval to tm");
 	}
 	else
 	{
@@ -5189,8 +5204,7 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
 	int			type,
 				val;
 	char	   *lowunits;
-	fsec_t		fsec;
-	struct pg_tm tt,
+	struct pg_itm tt,
 			   *tm = &tt;
 
 	lowunits = downcase_truncate_identifier(VARDATA_ANY(units),
@@ -5203,12 +5217,12 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
 
 	if (type == UNITS)
 	{
-		if (interval2tm(*interval, tm, &fsec) == 0)
+		interval2itm(*interval, tm);
 		{
 			switch (val)
 			{
 				case DTK_MICROSEC:
-					intresult = tm->tm_sec * INT64CONST(1000000) + fsec;
+					intresult = tm->tm_sec * INT64CONST(1000000) + tm->tm_usec;
 					break;
 
 				case DTK_MILLISEC:
@@ -5217,9 +5231,9 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
 						 * tm->tm_sec * 1000 + fsec / 1000
 						 * = (tm->tm_sec * 1'000'000 + fsec) / 1000
 						 */
-						PG_RETURN_NUMERIC(int64_div_fast_to_numeric(tm->tm_sec * INT64CONST(1000000) + fsec, 3));
+						PG_RETURN_NUMERIC(int64_div_fast_to_numeric(tm->tm_sec * INT64CONST(1000000) + tm->tm_usec, 3));
 					else
-						PG_RETURN_FLOAT8(tm->tm_sec * 1000.0 + fsec / 1000.0);
+						PG_RETURN_FLOAT8(tm->tm_sec * 1000.0 + tm->tm_usec / 1000.0);
 					break;
 
 				case DTK_SECOND:
@@ -5228,9 +5242,9 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
 						 * tm->tm_sec + fsec / 1'000'000
 						 * = (tm->tm_sec * 1'000'000 + fsec) / 1'000'000
 						 */
-						PG_RETURN_NUMERIC(int64_div_fast_to_numeric(tm->tm_sec * INT64CONST(1000000) + fsec, 6));
+						PG_RETURN_NUMERIC(int64_div_fast_to_numeric(tm->tm_sec * INT64CONST(1000000) + tm->tm_usec, 6));
 					else
-						PG_RETURN_FLOAT8(tm->tm_sec + fsec / 1000000.0);
+						PG_RETURN_FLOAT8(tm->tm_sec + tm->tm_usec / 1000000.0);
 					break;
 
 				case DTK_MINUTE:
@@ -5280,11 +5294,6 @@ interval_part_common(PG_FUNCTION_ARGS, bool retnumeric)
 					intresult = 0;
 			}
 		}
-		else
-		{
-			elog(ERROR, "could not convert interval to tm");
-			intresult = 0;
-		}
 	}
 	else if (type == RESERV && val == DTK_EPOCH)
 	{
diff --git a/src/include/datatype/timestamp.h b/src/include/datatype/timestamp.h
index 5fa38d20d8..d155f1b03b 100644
--- a/src/include/datatype/timestamp.h
+++ b/src/include/datatype/timestamp.h
@@ -40,6 +40,10 @@ typedef int64 TimestampTz;
 typedef int64 TimeOffset;
 typedef int32 fsec_t;			/* fractional seconds (in microseconds) */
 
+
+/*
+ * Storage format for type interval.
+ */
 typedef struct
 {
 	TimeOffset	time;			/* all time units other than days, months and
@@ -48,6 +52,41 @@ typedef struct
 	int32		month;			/* months and years, after time for alignment */
 } Interval;
 
+/*
+ * Data structure representing a broken-down interval.
+ *
+ * For historical reasons, this is modeled on struct pg_tm for timestamps.
+ * Unlike the situation for timestamps, there's no magic interpretation
+ * needed for months or years: they're just zero or not.  Note that fields
+ * can be negative; however, because of the divisions done while converting
+ * from struct Interval, only tm_mday could be INT_MIN.  This is important
+ * because we may need to negate the values in some code paths.
+ */
+struct pg_itm
+{
+	int			tm_usec;
+	int			tm_sec;
+	int			tm_min;
+	int64		tm_hour;		/* needs to be wide */
+	int			tm_mday;
+	int			tm_mon;
+	int			tm_year;
+};
+
+/*
+ * Data structure for decoding intervals.  We could just use struct pg_itm,
+ * but then the requirement for tm_usec to be 64 bits would propagate to
+ * places where it's not really needed.  Also, omitting the fields that
+ * aren't used during decoding seems like a good error-prevention measure.
+ */
+struct pg_itm_in
+{
+	int64		tm_usec;		/* needs to be wide */
+	int			tm_mday;
+	int			tm_mon;
+	int			tm_year;
+};
+
 
 /* Limits on the "precision" option (typmod) for these data types */
 #define MAX_TIMESTAMP_PRECISION 6
diff --git a/src/include/pgtime.h b/src/include/pgtime.h
index 2977b13aab..441d7847c1 100644
--- a/src/include/pgtime.h
+++ b/src/include/pgtime.h
@@ -23,6 +23,8 @@
 typedef int64 pg_time_t;
 
 /*
+ * Data structure representing a broken-down timestamp.
+ *
  * CAUTION: the IANA timezone library (src/timezone/) follows the POSIX
  * convention that tm_mon counts from 0 and tm_year is relative to 1900.
  * However, Postgres' datetime functions generally treat tm_mon as counting
@@ -44,6 +46,7 @@ struct pg_tm
 	const char *tm_zone;
 };
 
+/* These structs are opaque outside the timezone library */
 typedef struct pg_tz pg_tz;
 typedef struct pg_tzenum pg_tzenum;
 
diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h
index 0d158f3e4b..0801858d60 100644
--- a/src/include/utils/datetime.h
+++ b/src/include/utils/datetime.h
@@ -300,9 +300,9 @@ extern int	DecodeTimeOnly(char **field, int *ftype,
 						   int nf, int *dtype,
 						   struct pg_tm *tm, fsec_t *fsec, int *tzp);
 extern int	DecodeInterval(char **field, int *ftype, int nf, int range,
-						   int *dtype, struct pg_tm *tm, fsec_t *fsec);
+						   int *dtype, struct pg_itm_in *itm_in);
 extern int	DecodeISO8601Interval(char *str,
-								  int *dtype, struct pg_tm *tm, fsec_t *fsec);
+								  int *dtype, struct pg_itm_in *itm_in);
 
 extern void DateTimeParseError(int dterr, const char *str,
 							   const char *datatype) pg_attribute_noreturn();
@@ -315,7 +315,7 @@ extern int	DetermineTimeZoneAbbrevOffsetTS(TimestampTz ts, const char *abbr,
 extern void EncodeDateOnly(struct pg_tm *tm, int style, char *str);
 extern void EncodeTimeOnly(struct pg_tm *tm, fsec_t fsec, bool print_tz, int tz, int style, char *str);
 extern void EncodeDateTime(struct pg_tm *tm, fsec_t fsec, bool print_tz, int tz, const char *tzn, int style, char *str);
-extern void EncodeInterval(struct pg_tm *tm, fsec_t fsec, int style, char *str);
+extern void EncodeInterval(struct pg_itm *itm, int style, char *str);
 extern void EncodeSpecialTimestamp(Timestamp dt, char *str);
 
 extern int	ValidateDate(int fmask, bool isjulian, bool is2digits, bool bc,
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index c1a74f8e2b..d33421d380 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -88,8 +88,9 @@ extern int	timestamp2tm(Timestamp dt, int *tzp, struct pg_tm *tm,
 						 fsec_t *fsec, const char **tzn, pg_tz *attimezone);
 extern void dt2time(Timestamp dt, int *hour, int *min, int *sec, fsec_t *fsec);
 
-extern int	interval2tm(Interval span, struct pg_tm *tm, fsec_t *fsec);
-extern int	tm2interval(struct pg_tm *tm, fsec_t fsec, Interval *span);
+extern void interval2itm(Interval span, struct pg_itm *itm);
+extern int	itm2interval(struct pg_itm *itm, Interval *span);
+extern int	itmin2interval(struct pg_itm_in *itm_in, Interval *span);
 
 extern Timestamp SetEpochTimestamp(void);
 extern void GetEpochTime(struct pg_tm *tm);
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 146f7c55d0..00ffe0e2be 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -1079,3 +1079,614 @@ SELECT extract(epoch from interval '1000000000 days');
  86400000000000.000000
 (1 row)
 
+-- test time fields using entire 64 bit microseconds range
+SELECT INTERVAL '2562047788.01521550194 hours';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL '-2562047788.01521550222 hours';
+               interval                
+---------------------------------------
+ @ 2562047788 hours 54.775808 secs ago
+(1 row)
+
+SELECT INTERVAL '153722867280.912930117 minutes';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL '-153722867280.912930133 minutes';
+               interval                
+---------------------------------------
+ @ 2562047788 hours 54.775808 secs ago
+(1 row)
+
+SELECT INTERVAL '9223372036854.775807 seconds';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL '-9223372036854.775808 seconds';
+               interval                
+---------------------------------------
+ @ 2562047788 hours 54.775808 secs ago
+(1 row)
+
+SELECT INTERVAL '9223372036854775.807 milliseconds';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL '-9223372036854775.808 milliseconds';
+               interval                
+---------------------------------------
+ @ 2562047788 hours 54.775808 secs ago
+(1 row)
+
+SELECT INTERVAL '9223372036854775807 microseconds';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL '-9223372036854775808 microseconds';
+               interval                
+---------------------------------------
+ @ 2562047788 hours 54.775808 secs ago
+(1 row)
+
+SELECT INTERVAL 'PT2562047788H54.775807S';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL 'PT-2562047788H-54.775808S';
+               interval                
+---------------------------------------
+ @ 2562047788 hours 54.775808 secs ago
+(1 row)
+
+SELECT INTERVAL 'PT2562047788:00:54.775807';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL 'PT2562047788.0152155019444';
+             interval              
+-----------------------------------
+ @ 2562047788 hours 54.775807 secs
+(1 row)
+
+SELECT INTERVAL 'PT-2562047788.0152155022222';
+               interval                
+---------------------------------------
+ @ 2562047788 hours 54.775808 secs ago
+(1 row)
+
+-- overflow each date/time field
+SELECT INTERVAL '2147483648 years';
+ERROR:  interval field value out of range: "2147483648 years"
+LINE 1: SELECT INTERVAL '2147483648 years';
+                        ^
+SELECT INTERVAL '-2147483649 years';
+ERROR:  interval field value out of range: "-2147483649 years"
+LINE 1: SELECT INTERVAL '-2147483649 years';
+                        ^
+SELECT INTERVAL '2147483648 months';
+ERROR:  interval field value out of range: "2147483648 months"
+LINE 1: SELECT INTERVAL '2147483648 months';
+                        ^
+SELECT INTERVAL '-2147483649 months';
+ERROR:  interval field value out of range: "-2147483649 months"
+LINE 1: SELECT INTERVAL '-2147483649 months';
+                        ^
+SELECT INTERVAL '2147483648 days';
+ERROR:  interval field value out of range: "2147483648 days"
+LINE 1: SELECT INTERVAL '2147483648 days';
+                        ^
+SELECT INTERVAL '-2147483649 days';
+ERROR:  interval field value out of range: "-2147483649 days"
+LINE 1: SELECT INTERVAL '-2147483649 days';
+                        ^
+SELECT INTERVAL '2562047789 hours';
+ERROR:  interval field value out of range: "2562047789 hours"
+LINE 1: SELECT INTERVAL '2562047789 hours';
+                        ^
+SELECT INTERVAL '-2562047789 hours';
+ERROR:  interval field value out of range: "-2562047789 hours"
+LINE 1: SELECT INTERVAL '-2562047789 hours';
+                        ^
+SELECT INTERVAL '153722867281 minutes';
+ERROR:  interval field value out of range: "153722867281 minutes"
+LINE 1: SELECT INTERVAL '153722867281 minutes';
+                        ^
+SELECT INTERVAL '-153722867281 minutes';
+ERROR:  interval field value out of range: "-153722867281 minutes"
+LINE 1: SELECT INTERVAL '-153722867281 minutes';
+                        ^
+SELECT INTERVAL '9223372036855 seconds';
+ERROR:  interval field value out of range: "9223372036855 seconds"
+LINE 1: SELECT INTERVAL '9223372036855 seconds';
+                        ^
+SELECT INTERVAL '-9223372036855 seconds';
+ERROR:  interval field value out of range: "-9223372036855 seconds"
+LINE 1: SELECT INTERVAL '-9223372036855 seconds';
+                        ^
+SELECT INTERVAL '9223372036854777 millisecond';
+ERROR:  interval field value out of range: "9223372036854777 millisecond"
+LINE 1: SELECT INTERVAL '9223372036854777 millisecond';
+                        ^
+SELECT INTERVAL '-9223372036854777 millisecond';
+ERROR:  interval field value out of range: "-9223372036854777 millisecond"
+LINE 1: SELECT INTERVAL '-9223372036854777 millisecond';
+                        ^
+SELECT INTERVAL '9223372036854775808 microsecond';
+ERROR:  interval field value out of range: "9223372036854775808 microsecond"
+LINE 1: SELECT INTERVAL '9223372036854775808 microsecond';
+                        ^
+SELECT INTERVAL '-9223372036854775809 microsecond';
+ERROR:  interval field value out of range: "-9223372036854775809 microsecond"
+LINE 1: SELECT INTERVAL '-9223372036854775809 microsecond';
+                        ^
+SELECT INTERVAL 'P2147483648';
+ERROR:  interval field value out of range: "P2147483648"
+LINE 1: SELECT INTERVAL 'P2147483648';
+                        ^
+SELECT INTERVAL 'P-2147483649';
+ERROR:  interval field value out of range: "P-2147483649"
+LINE 1: SELECT INTERVAL 'P-2147483649';
+                        ^
+SELECT INTERVAL 'P1-2147483647-2147483647';
+ERROR:  interval out of range
+LINE 1: SELECT INTERVAL 'P1-2147483647-2147483647';
+                        ^
+SELECT INTERVAL 'PT2562047789';
+ERROR:  interval field value out of range: "PT2562047789"
+LINE 1: SELECT INTERVAL 'PT2562047789';
+                        ^
+SELECT INTERVAL 'PT-2562047789';
+ERROR:  interval field value out of range: "PT-2562047789"
+LINE 1: SELECT INTERVAL 'PT-2562047789';
+                        ^
+-- overflow with date/time unit aliases
+SELECT INTERVAL '2147483647 weeks';
+ERROR:  interval field value out of range: "2147483647 weeks"
+LINE 1: SELECT INTERVAL '2147483647 weeks';
+                        ^
+SELECT INTERVAL '-2147483648 weeks';
+ERROR:  interval field value out of range: "-2147483648 weeks"
+LINE 1: SELECT INTERVAL '-2147483648 weeks';
+                        ^
+SELECT INTERVAL '2147483647 decades';
+ERROR:  interval field value out of range: "2147483647 decades"
+LINE 1: SELECT INTERVAL '2147483647 decades';
+                        ^
+SELECT INTERVAL '-2147483648 decades';
+ERROR:  interval field value out of range: "-2147483648 decades"
+LINE 1: SELECT INTERVAL '-2147483648 decades';
+                        ^
+SELECT INTERVAL '2147483647 centuries';
+ERROR:  interval field value out of range: "2147483647 centuries"
+LINE 1: SELECT INTERVAL '2147483647 centuries';
+                        ^
+SELECT INTERVAL '-2147483648 centuries';
+ERROR:  interval field value out of range: "-2147483648 centuries"
+LINE 1: SELECT INTERVAL '-2147483648 centuries';
+                        ^
+SELECT INTERVAL '2147483647 millennium';
+ERROR:  interval field value out of range: "2147483647 millennium"
+LINE 1: SELECT INTERVAL '2147483647 millennium';
+                        ^
+SELECT INTERVAL '-2147483648 millennium';
+ERROR:  interval field value out of range: "-2147483648 millennium"
+LINE 1: SELECT INTERVAL '-2147483648 millennium';
+                        ^
+SELECT INTERVAL '1 week 2147483647 days';
+ERROR:  interval field value out of range: "1 week 2147483647 days"
+LINE 1: SELECT INTERVAL '1 week 2147483647 days';
+                        ^
+SELECT INTERVAL '-1 week -2147483648 days';
+ERROR:  interval field value out of range: "-1 week -2147483648 days"
+LINE 1: SELECT INTERVAL '-1 week -2147483648 days';
+                        ^
+SELECT INTERVAL '2147483647 days 1 week';
+ERROR:  interval field value out of range: "2147483647 days 1 week"
+LINE 1: SELECT INTERVAL '2147483647 days 1 week';
+                        ^
+SELECT INTERVAL '-2147483648 days -1 week';
+ERROR:  interval field value out of range: "-2147483648 days -1 week"
+LINE 1: SELECT INTERVAL '-2147483648 days -1 week';
+                        ^
+SELECT INTERVAL 'P1W2147483647D';
+ERROR:  interval field value out of range: "P1W2147483647D"
+LINE 1: SELECT INTERVAL 'P1W2147483647D';
+                        ^
+SELECT INTERVAL 'P-1W-2147483648D';
+ERROR:  interval field value out of range: "P-1W-2147483648D"
+LINE 1: SELECT INTERVAL 'P-1W-2147483648D';
+                        ^
+SELECT INTERVAL 'P2147483647D1W';
+ERROR:  interval field value out of range: "P2147483647D1W"
+LINE 1: SELECT INTERVAL 'P2147483647D1W';
+                        ^
+SELECT INTERVAL 'P-2147483648D-1W';
+ERROR:  interval field value out of range: "P-2147483648D-1W"
+LINE 1: SELECT INTERVAL 'P-2147483648D-1W';
+                        ^
+SELECT INTERVAL '1 decade 2147483647 years';
+ERROR:  interval field value out of range: "1 decade 2147483647 years"
+LINE 1: SELECT INTERVAL '1 decade 2147483647 years';
+                        ^
+SELECT INTERVAL '1 century 2147483647 years';
+ERROR:  interval field value out of range: "1 century 2147483647 years"
+LINE 1: SELECT INTERVAL '1 century 2147483647 years';
+                        ^
+SELECT INTERVAL '1 millennium 2147483647 years';
+ERROR:  interval field value out of range: "1 millennium 2147483647 years"
+LINE 1: SELECT INTERVAL '1 millennium 2147483647 years';
+                        ^
+SELECT INTERVAL '-1 decade -2147483648 years';
+ERROR:  interval field value out of range: "-1 decade -2147483648 years"
+LINE 1: SELECT INTERVAL '-1 decade -2147483648 years';
+                        ^
+SELECT INTERVAL '-1 century -2147483648 years';
+ERROR:  interval field value out of range: "-1 century -2147483648 years"
+LINE 1: SELECT INTERVAL '-1 century -2147483648 years';
+                        ^
+SELECT INTERVAL '-1 millennium -2147483648 years';
+ERROR:  interval field value out of range: "-1 millennium -2147483648 years"
+LINE 1: SELECT INTERVAL '-1 millennium -2147483648 years';
+                        ^
+SELECT INTERVAL '2147483647 years 1 decade';
+ERROR:  interval field value out of range: "2147483647 years 1 decade"
+LINE 1: SELECT INTERVAL '2147483647 years 1 decade';
+                        ^
+SELECT INTERVAL '2147483647 years 1 century';
+ERROR:  interval field value out of range: "2147483647 years 1 century"
+LINE 1: SELECT INTERVAL '2147483647 years 1 century';
+                        ^
+SELECT INTERVAL '2147483647 years 1 millennium';
+ERROR:  interval field value out of range: "2147483647 years 1 millennium"
+LINE 1: SELECT INTERVAL '2147483647 years 1 millennium';
+                        ^
+SELECT INTERVAL '-2147483648 years -1 decade';
+ERROR:  interval field value out of range: "-2147483648 years -1 decade"
+LINE 1: SELECT INTERVAL '-2147483648 years -1 decade';
+                        ^
+SELECT INTERVAL '-2147483648 years -1 century';
+ERROR:  interval field value out of range: "-2147483648 years -1 century"
+LINE 1: SELECT INTERVAL '-2147483648 years -1 century';
+                        ^
+SELECT INTERVAL '-2147483648 years -1 millennium';
+ERROR:  interval field value out of range: "-2147483648 years -1 millennium"
+LINE 1: SELECT INTERVAL '-2147483648 years -1 millennium';
+                        ^
+-- overflowing with fractional fields - postgres format
+SELECT INTERVAL '0.1 millennium 2147483647 months';
+ERROR:  interval field value out of range: "0.1 millennium 2147483647 months"
+LINE 1: SELECT INTERVAL '0.1 millennium 2147483647 months';
+                        ^
+SELECT INTERVAL '0.1 centuries 2147483647 months';
+ERROR:  interval field value out of range: "0.1 centuries 2147483647 months"
+LINE 1: SELECT INTERVAL '0.1 centuries 2147483647 months';
+                        ^
+SELECT INTERVAL '0.1 decades 2147483647 months';
+ERROR:  interval field value out of range: "0.1 decades 2147483647 months"
+LINE 1: SELECT INTERVAL '0.1 decades 2147483647 months';
+                        ^
+SELECT INTERVAL '0.1 yrs 2147483647 months';
+ERROR:  interval field value out of range: "0.1 yrs 2147483647 months"
+LINE 1: SELECT INTERVAL '0.1 yrs 2147483647 months';
+                        ^
+SELECT INTERVAL '-0.1 millennium -2147483648 months';
+ERROR:  interval field value out of range: "-0.1 millennium -2147483648 months"
+LINE 1: SELECT INTERVAL '-0.1 millennium -2147483648 months';
+                        ^
+SELECT INTERVAL '-0.1 centuries -2147483648 months';
+ERROR:  interval field value out of range: "-0.1 centuries -2147483648 months"
+LINE 1: SELECT INTERVAL '-0.1 centuries -2147483648 months';
+                        ^
+SELECT INTERVAL '-0.1 decades -2147483648 months';
+ERROR:  interval field value out of range: "-0.1 decades -2147483648 months"
+LINE 1: SELECT INTERVAL '-0.1 decades -2147483648 months';
+                        ^
+SELECT INTERVAL '-0.1 yrs -2147483648 months';
+ERROR:  interval field value out of range: "-0.1 yrs -2147483648 months"
+LINE 1: SELECT INTERVAL '-0.1 yrs -2147483648 months';
+                        ^
+SELECT INTERVAL '2147483647 months 0.1 millennium';
+ERROR:  interval field value out of range: "2147483647 months 0.1 millennium"
+LINE 1: SELECT INTERVAL '2147483647 months 0.1 millennium';
+                        ^
+SELECT INTERVAL '2147483647 months 0.1 centuries';
+ERROR:  interval field value out of range: "2147483647 months 0.1 centuries"
+LINE 1: SELECT INTERVAL '2147483647 months 0.1 centuries';
+                        ^
+SELECT INTERVAL '2147483647 months 0.1 decades';
+ERROR:  interval field value out of range: "2147483647 months 0.1 decades"
+LINE 1: SELECT INTERVAL '2147483647 months 0.1 decades';
+                        ^
+SELECT INTERVAL '2147483647 months 0.1 yrs';
+ERROR:  interval field value out of range: "2147483647 months 0.1 yrs"
+LINE 1: SELECT INTERVAL '2147483647 months 0.1 yrs';
+                        ^
+SELECT INTERVAL '-2147483648 months -0.1 millennium';
+ERROR:  interval field value out of range: "-2147483648 months -0.1 millennium"
+LINE 1: SELECT INTERVAL '-2147483648 months -0.1 millennium';
+                        ^
+SELECT INTERVAL '-2147483648 months -0.1 centuries';
+ERROR:  interval field value out of range: "-2147483648 months -0.1 centuries"
+LINE 1: SELECT INTERVAL '-2147483648 months -0.1 centuries';
+                        ^
+SELECT INTERVAL '-2147483648 months -0.1 decades';
+ERROR:  interval field value out of range: "-2147483648 months -0.1 decades"
+LINE 1: SELECT INTERVAL '-2147483648 months -0.1 decades';
+                        ^
+SELECT INTERVAL '-2147483648 months -0.1 yrs';
+ERROR:  interval field value out of range: "-2147483648 months -0.1 yrs"
+LINE 1: SELECT INTERVAL '-2147483648 months -0.1 yrs';
+                        ^
+SELECT INTERVAL '0.1 months 2147483647 days';
+ERROR:  interval field value out of range: "0.1 months 2147483647 days"
+LINE 1: SELECT INTERVAL '0.1 months 2147483647 days';
+                        ^
+SELECT INTERVAL '-0.1 months -2147483648 days';
+ERROR:  interval field value out of range: "-0.1 months -2147483648 days"
+LINE 1: SELECT INTERVAL '-0.1 months -2147483648 days';
+                        ^
+SELECT INTERVAL '2147483647 days 0.1 months';
+ERROR:  interval field value out of range: "2147483647 days 0.1 months"
+LINE 1: SELECT INTERVAL '2147483647 days 0.1 months';
+                        ^
+SELECT INTERVAL '-2147483648 days -0.1 months';
+ERROR:  interval field value out of range: "-2147483648 days -0.1 months"
+LINE 1: SELECT INTERVAL '-2147483648 days -0.1 months';
+                        ^
+SELECT INTERVAL '0.5 weeks 2147483647 days';
+ERROR:  interval field value out of range: "0.5 weeks 2147483647 days"
+LINE 1: SELECT INTERVAL '0.5 weeks 2147483647 days';
+                        ^
+SELECT INTERVAL '-0.5 weeks -2147483648 days';
+ERROR:  interval field value out of range: "-0.5 weeks -2147483648 days"
+LINE 1: SELECT INTERVAL '-0.5 weeks -2147483648 days';
+                        ^
+SELECT INTERVAL '2147483647 days 0.5 weeks';
+ERROR:  interval field value out of range: "2147483647 days 0.5 weeks"
+LINE 1: SELECT INTERVAL '2147483647 days 0.5 weeks';
+                        ^
+SELECT INTERVAL '-2147483648 days -0.5 weeks';
+ERROR:  interval field value out of range: "-2147483648 days -0.5 weeks"
+LINE 1: SELECT INTERVAL '-2147483648 days -0.5 weeks';
+                        ^
+SELECT INTERVAL '0.01 months 9223372036854775807 microseconds';
+ERROR:  interval field value out of range: "0.01 months 9223372036854775807 microseconds"
+LINE 1: SELECT INTERVAL '0.01 months 9223372036854775807 microsecond...
+                        ^
+SELECT INTERVAL '-0.01 months -9223372036854775808 microseconds';
+ERROR:  interval field value out of range: "-0.01 months -9223372036854775808 microseconds"
+LINE 1: SELECT INTERVAL '-0.01 months -9223372036854775808 microseco...
+                        ^
+SELECT INTERVAL '9223372036854775807 microseconds 0.01 months';
+ERROR:  interval field value out of range: "9223372036854775807 microseconds 0.01 months"
+LINE 1: SELECT INTERVAL '9223372036854775807 microseconds 0.01 month...
+                        ^
+SELECT INTERVAL '-9223372036854775808 microseconds -0.01 months';
+ERROR:  interval field value out of range: "-9223372036854775808 microseconds -0.01 months"
+LINE 1: SELECT INTERVAL '-9223372036854775808 microseconds -0.01 mon...
+                        ^
+SELECT INTERVAL '0.1 weeks 9223372036854775807 microseconds';
+ERROR:  interval field value out of range: "0.1 weeks 9223372036854775807 microseconds"
+LINE 1: SELECT INTERVAL '0.1 weeks 9223372036854775807 microseconds'...
+                        ^
+SELECT INTERVAL '-0.1 weeks -9223372036854775808 microseconds';
+ERROR:  interval field value out of range: "-0.1 weeks -9223372036854775808 microseconds"
+LINE 1: SELECT INTERVAL '-0.1 weeks -9223372036854775808 microsecond...
+                        ^
+SELECT INTERVAL '9223372036854775807 microseconds 0.1 weeks';
+ERROR:  interval field value out of range: "9223372036854775807 microseconds 0.1 weeks"
+LINE 1: SELECT INTERVAL '9223372036854775807 microseconds 0.1 weeks'...
+                        ^
+SELECT INTERVAL '-9223372036854775808 microseconds -0.1 weeks';
+ERROR:  interval field value out of range: "-9223372036854775808 microseconds -0.1 weeks"
+LINE 1: SELECT INTERVAL '-9223372036854775808 microseconds -0.1 week...
+                        ^
+SELECT INTERVAL '0.1 days 9223372036854775807 microseconds';
+ERROR:  interval field value out of range: "0.1 days 9223372036854775807 microseconds"
+LINE 1: SELECT INTERVAL '0.1 days 9223372036854775807 microseconds';
+                        ^
+SELECT INTERVAL '-0.1 days -9223372036854775808 microseconds';
+ERROR:  interval field value out of range: "-0.1 days -9223372036854775808 microseconds"
+LINE 1: SELECT INTERVAL '-0.1 days -9223372036854775808 microseconds...
+                        ^
+SELECT INTERVAL '9223372036854775807 microseconds 0.1 days';
+ERROR:  interval field value out of range: "9223372036854775807 microseconds 0.1 days"
+LINE 1: SELECT INTERVAL '9223372036854775807 microseconds 0.1 days';
+                        ^
+SELECT INTERVAL '-9223372036854775808 microseconds -0.1 days';
+ERROR:  interval field value out of range: "-9223372036854775808 microseconds -0.1 days"
+LINE 1: SELECT INTERVAL '-9223372036854775808 microseconds -0.1 days...
+                        ^
+-- overflowing with fractional fields - ISO8601 format
+SELECT INTERVAL 'P0.1Y2147483647M';
+ERROR:  interval field value out of range: "P0.1Y2147483647M"
+LINE 1: SELECT INTERVAL 'P0.1Y2147483647M';
+                        ^
+SELECT INTERVAL 'P-0.1Y-2147483648M';
+ERROR:  interval field value out of range: "P-0.1Y-2147483648M"
+LINE 1: SELECT INTERVAL 'P-0.1Y-2147483648M';
+                        ^
+SELECT INTERVAL 'P2147483647M0.1Y';
+ERROR:  interval field value out of range: "P2147483647M0.1Y"
+LINE 1: SELECT INTERVAL 'P2147483647M0.1Y';
+                        ^
+SELECT INTERVAL 'P-2147483648M-0.1Y';
+ERROR:  interval field value out of range: "P-2147483648M-0.1Y"
+LINE 1: SELECT INTERVAL 'P-2147483648M-0.1Y';
+                        ^
+SELECT INTERVAL 'P0.1M2147483647D';
+ERROR:  interval field value out of range: "P0.1M2147483647D"
+LINE 1: SELECT INTERVAL 'P0.1M2147483647D';
+                        ^
+SELECT INTERVAL 'P-0.1M-2147483648D';
+ERROR:  interval field value out of range: "P-0.1M-2147483648D"
+LINE 1: SELECT INTERVAL 'P-0.1M-2147483648D';
+                        ^
+SELECT INTERVAL 'P2147483647D0.1M';
+ERROR:  interval field value out of range: "P2147483647D0.1M"
+LINE 1: SELECT INTERVAL 'P2147483647D0.1M';
+                        ^
+SELECT INTERVAL 'P-2147483648D-0.1M';
+ERROR:  interval field value out of range: "P-2147483648D-0.1M"
+LINE 1: SELECT INTERVAL 'P-2147483648D-0.1M';
+                        ^
+SELECT INTERVAL 'P0.5W2147483647D';
+ERROR:  interval field value out of range: "P0.5W2147483647D"
+LINE 1: SELECT INTERVAL 'P0.5W2147483647D';
+                        ^
+SELECT INTERVAL 'P-0.5W-2147483648D';
+ERROR:  interval field value out of range: "P-0.5W-2147483648D"
+LINE 1: SELECT INTERVAL 'P-0.5W-2147483648D';
+                        ^
+SELECT INTERVAL 'P2147483647D0.5W';
+ERROR:  interval field value out of range: "P2147483647D0.5W"
+LINE 1: SELECT INTERVAL 'P2147483647D0.5W';
+                        ^
+SELECT INTERVAL 'P-2147483648D-0.5W';
+ERROR:  interval field value out of range: "P-2147483648D-0.5W"
+LINE 1: SELECT INTERVAL 'P-2147483648D-0.5W';
+                        ^
+SELECT INTERVAL 'P0.01MT2562047788H54.775807S';
+ERROR:  interval field value out of range: "P0.01MT2562047788H54.775807S"
+LINE 1: SELECT INTERVAL 'P0.01MT2562047788H54.775807S';
+                        ^
+SELECT INTERVAL 'P-0.01MT-2562047788H-54.775808S';
+ERROR:  interval field value out of range: "P-0.01MT-2562047788H-54.775808S"
+LINE 1: SELECT INTERVAL 'P-0.01MT-2562047788H-54.775808S';
+                        ^
+SELECT INTERVAL 'P0.1DT2562047788H54.775807S';
+ERROR:  interval field value out of range: "P0.1DT2562047788H54.775807S"
+LINE 1: SELECT INTERVAL 'P0.1DT2562047788H54.775807S';
+                        ^
+SELECT INTERVAL 'P-0.1DT-2562047788H-54.775808S';
+ERROR:  interval field value out of range: "P-0.1DT-2562047788H-54.775808S"
+LINE 1: SELECT INTERVAL 'P-0.1DT-2562047788H-54.775808S';
+                        ^
+SELECT INTERVAL 'PT2562047788.1H54.775807S';
+ERROR:  interval field value out of range: "PT2562047788.1H54.775807S"
+LINE 1: SELECT INTERVAL 'PT2562047788.1H54.775807S';
+                        ^
+SELECT INTERVAL 'PT-2562047788.1H-54.775808S';
+ERROR:  interval field value out of range: "PT-2562047788.1H-54.775808S"
+LINE 1: SELECT INTERVAL 'PT-2562047788.1H-54.775808S';
+                        ^
+SELECT INTERVAL 'PT2562047788H0.1M54.775807S';
+ERROR:  interval field value out of range: "PT2562047788H0.1M54.775807S"
+LINE 1: SELECT INTERVAL 'PT2562047788H0.1M54.775807S';
+                        ^
+SELECT INTERVAL 'PT-2562047788H-0.1M-54.775808S';
+ERROR:  interval field value out of range: "PT-2562047788H-0.1M-54.775808S"
+LINE 1: SELECT INTERVAL 'PT-2562047788H-0.1M-54.775808S';
+                        ^
+-- overflowing with fractional fields - ISO8601 alternative format
+SELECT INTERVAL 'P0.1-2147483647-00';
+ERROR:  interval field value out of range: "P0.1-2147483647-00"
+LINE 1: SELECT INTERVAL 'P0.1-2147483647-00';
+                        ^
+SELECT INTERVAL 'P00-0.1-2147483647';
+ERROR:  interval field value out of range: "P00-0.1-2147483647"
+LINE 1: SELECT INTERVAL 'P00-0.1-2147483647';
+                        ^
+SELECT INTERVAL 'P00-0.01-00T2562047788:00:54.775807';
+ERROR:  interval field value out of range: "P00-0.01-00T2562047788:00:54.775807"
+LINE 1: SELECT INTERVAL 'P00-0.01-00T2562047788:00:54.775807';
+                        ^
+SELECT INTERVAL 'P00-00-0.1T2562047788:00:54.775807';
+ERROR:  interval field value out of range: "P00-00-0.1T2562047788:00:54.775807"
+LINE 1: SELECT INTERVAL 'P00-00-0.1T2562047788:00:54.775807';
+                        ^
+SELECT INTERVAL 'PT2562047788.1:00:54.775807';
+ERROR:  interval field value out of range: "PT2562047788.1:00:54.775807"
+LINE 1: SELECT INTERVAL 'PT2562047788.1:00:54.775807';
+                        ^
+SELECT INTERVAL 'PT2562047788:01.:54.775807';
+ERROR:  interval field value out of range: "PT2562047788:01.:54.775807"
+LINE 1: SELECT INTERVAL 'PT2562047788:01.:54.775807';
+                        ^
+-- overflowing with fractional fields - SQL standard format
+SELECT INTERVAL '0.1 2562047788:0:54.775807';
+ERROR:  interval field value out of range: "0.1 2562047788:0:54.775807"
+LINE 1: SELECT INTERVAL '0.1 2562047788:0:54.775807';
+                        ^
+SELECT INTERVAL '0.1 2562047788:0:54.775808 ago';
+ERROR:  interval field value out of range: "0.1 2562047788:0:54.775808 ago"
+LINE 1: SELECT INTERVAL '0.1 2562047788:0:54.775808 ago';
+                        ^
+SELECT INTERVAL '2562047788.1:0:54.775807';
+ERROR:  interval field value out of range: "2562047788.1:0:54.775807"
+LINE 1: SELECT INTERVAL '2562047788.1:0:54.775807';
+                        ^
+SELECT INTERVAL '2562047788.1:0:54.775808 ago';
+ERROR:  interval field value out of range: "2562047788.1:0:54.775808 ago"
+LINE 1: SELECT INTERVAL '2562047788.1:0:54.775808 ago';
+                        ^
+SELECT INTERVAL '2562047788:0.1:54.775807';
+ERROR:  invalid input syntax for type interval: "2562047788:0.1:54.775807"
+LINE 1: SELECT INTERVAL '2562047788:0.1:54.775807';
+                        ^
+SELECT INTERVAL '2562047788:0.1:54.775808 ago';
+ERROR:  invalid input syntax for type interval: "2562047788:0.1:54.775808 ago"
+LINE 1: SELECT INTERVAL '2562047788:0.1:54.775808 ago';
+                        ^
+-- overflowing using AGO with INT_MIN
+SELECT INTERVAL '-2147483648 months ago';
+ERROR:  interval field value out of range: "-2147483648 months ago"
+LINE 1: SELECT INTERVAL '-2147483648 months ago';
+                        ^
+SELECT INTERVAL '-2147483648 days ago';
+ERROR:  interval field value out of range: "-2147483648 days ago"
+LINE 1: SELECT INTERVAL '-2147483648 days ago';
+                        ^
+SELECT INTERVAL '-9223372036854775808 microseconds ago';
+ERROR:  interval field value out of range: "-9223372036854775808 microseconds ago"
+LINE 1: SELECT INTERVAL '-9223372036854775808 microseconds ago';
+                        ^
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 microseconds ago';
+ERROR:  interval field value out of range: "-2147483648 months -2147483648 days -9223372036854775808 microseconds ago"
+LINE 1: SELECT INTERVAL '-2147483648 months -2147483648 days -922337...
+                        ^
+-- test that INT_MIN number is formatted properly
+SET IntervalStyle to postgres;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+                              interval                              
+--------------------------------------------------------------------
+ -178956970 years -8 mons -2147483648 days -2562047788:00:54.775808
+(1 row)
+
+SET IntervalStyle to postgres_verbose;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+                                   interval                                   
+------------------------------------------------------------------------------
+ @ 178956970 years 8 mons 2147483648 days 2562047788 hours 54.775808 secs ago
+(1 row)
+
+SET IntervalStyle TO sql_standard;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+                     interval                      
+---------------------------------------------------
+ -178956970-8 -2147483648 -2562047788:00:54.775808
+(1 row)
+
+SET IntervalStyle to iso_8601;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+                      interval                       
+-----------------------------------------------------
+ P-178956970Y-8M-2147483648DT-2562047788H-54.775808S
+(1 row)
+
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index c31f0eec05..fc924d5bde 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -367,3 +367,187 @@ SELECT f1,
 
 -- internal overflow test case
 SELECT extract(epoch from interval '1000000000 days');
+
+-- test time fields using entire 64 bit microseconds range
+SELECT INTERVAL '2562047788.01521550194 hours';
+SELECT INTERVAL '-2562047788.01521550222 hours';
+SELECT INTERVAL '153722867280.912930117 minutes';
+SELECT INTERVAL '-153722867280.912930133 minutes';
+SELECT INTERVAL '9223372036854.775807 seconds';
+SELECT INTERVAL '-9223372036854.775808 seconds';
+SELECT INTERVAL '9223372036854775.807 milliseconds';
+SELECT INTERVAL '-9223372036854775.808 milliseconds';
+SELECT INTERVAL '9223372036854775807 microseconds';
+SELECT INTERVAL '-9223372036854775808 microseconds';
+
+SELECT INTERVAL 'PT2562047788H54.775807S';
+SELECT INTERVAL 'PT-2562047788H-54.775808S';
+
+SELECT INTERVAL 'PT2562047788:00:54.775807';
+
+SELECT INTERVAL 'PT2562047788.0152155019444';
+SELECT INTERVAL 'PT-2562047788.0152155022222';
+
+-- overflow each date/time field
+SELECT INTERVAL '2147483648 years';
+SELECT INTERVAL '-2147483649 years';
+SELECT INTERVAL '2147483648 months';
+SELECT INTERVAL '-2147483649 months';
+SELECT INTERVAL '2147483648 days';
+SELECT INTERVAL '-2147483649 days';
+SELECT INTERVAL '2562047789 hours';
+SELECT INTERVAL '-2562047789 hours';
+SELECT INTERVAL '153722867281 minutes';
+SELECT INTERVAL '-153722867281 minutes';
+SELECT INTERVAL '9223372036855 seconds';
+SELECT INTERVAL '-9223372036855 seconds';
+SELECT INTERVAL '9223372036854777 millisecond';
+SELECT INTERVAL '-9223372036854777 millisecond';
+SELECT INTERVAL '9223372036854775808 microsecond';
+SELECT INTERVAL '-9223372036854775809 microsecond';
+
+SELECT INTERVAL 'P2147483648';
+SELECT INTERVAL 'P-2147483649';
+SELECT INTERVAL 'P1-2147483647-2147483647';
+SELECT INTERVAL 'PT2562047789';
+SELECT INTERVAL 'PT-2562047789';
+
+-- overflow with date/time unit aliases
+SELECT INTERVAL '2147483647 weeks';
+SELECT INTERVAL '-2147483648 weeks';
+SELECT INTERVAL '2147483647 decades';
+SELECT INTERVAL '-2147483648 decades';
+SELECT INTERVAL '2147483647 centuries';
+SELECT INTERVAL '-2147483648 centuries';
+SELECT INTERVAL '2147483647 millennium';
+SELECT INTERVAL '-2147483648 millennium';
+
+SELECT INTERVAL '1 week 2147483647 days';
+SELECT INTERVAL '-1 week -2147483648 days';
+SELECT INTERVAL '2147483647 days 1 week';
+SELECT INTERVAL '-2147483648 days -1 week';
+
+SELECT INTERVAL 'P1W2147483647D';
+SELECT INTERVAL 'P-1W-2147483648D';
+SELECT INTERVAL 'P2147483647D1W';
+SELECT INTERVAL 'P-2147483648D-1W';
+
+SELECT INTERVAL '1 decade 2147483647 years';
+SELECT INTERVAL '1 century 2147483647 years';
+SELECT INTERVAL '1 millennium 2147483647 years';
+SELECT INTERVAL '-1 decade -2147483648 years';
+SELECT INTERVAL '-1 century -2147483648 years';
+SELECT INTERVAL '-1 millennium -2147483648 years';
+
+SELECT INTERVAL '2147483647 years 1 decade';
+SELECT INTERVAL '2147483647 years 1 century';
+SELECT INTERVAL '2147483647 years 1 millennium';
+SELECT INTERVAL '-2147483648 years -1 decade';
+SELECT INTERVAL '-2147483648 years -1 century';
+SELECT INTERVAL '-2147483648 years -1 millennium';
+
+-- overflowing with fractional fields - postgres format
+SELECT INTERVAL '0.1 millennium 2147483647 months';
+SELECT INTERVAL '0.1 centuries 2147483647 months';
+SELECT INTERVAL '0.1 decades 2147483647 months';
+SELECT INTERVAL '0.1 yrs 2147483647 months';
+SELECT INTERVAL '-0.1 millennium -2147483648 months';
+SELECT INTERVAL '-0.1 centuries -2147483648 months';
+SELECT INTERVAL '-0.1 decades -2147483648 months';
+SELECT INTERVAL '-0.1 yrs -2147483648 months';
+
+SELECT INTERVAL '2147483647 months 0.1 millennium';
+SELECT INTERVAL '2147483647 months 0.1 centuries';
+SELECT INTERVAL '2147483647 months 0.1 decades';
+SELECT INTERVAL '2147483647 months 0.1 yrs';
+SELECT INTERVAL '-2147483648 months -0.1 millennium';
+SELECT INTERVAL '-2147483648 months -0.1 centuries';
+SELECT INTERVAL '-2147483648 months -0.1 decades';
+SELECT INTERVAL '-2147483648 months -0.1 yrs';
+
+SELECT INTERVAL '0.1 months 2147483647 days';
+SELECT INTERVAL '-0.1 months -2147483648 days';
+SELECT INTERVAL '2147483647 days 0.1 months';
+SELECT INTERVAL '-2147483648 days -0.1 months';
+
+SELECT INTERVAL '0.5 weeks 2147483647 days';
+SELECT INTERVAL '-0.5 weeks -2147483648 days';
+SELECT INTERVAL '2147483647 days 0.5 weeks';
+SELECT INTERVAL '-2147483648 days -0.5 weeks';
+
+SELECT INTERVAL '0.01 months 9223372036854775807 microseconds';
+SELECT INTERVAL '-0.01 months -9223372036854775808 microseconds';
+SELECT INTERVAL '9223372036854775807 microseconds 0.01 months';
+SELECT INTERVAL '-9223372036854775808 microseconds -0.01 months';
+
+SELECT INTERVAL '0.1 weeks 9223372036854775807 microseconds';
+SELECT INTERVAL '-0.1 weeks -9223372036854775808 microseconds';
+SELECT INTERVAL '9223372036854775807 microseconds 0.1 weeks';
+SELECT INTERVAL '-9223372036854775808 microseconds -0.1 weeks';
+
+SELECT INTERVAL '0.1 days 9223372036854775807 microseconds';
+SELECT INTERVAL '-0.1 days -9223372036854775808 microseconds';
+SELECT INTERVAL '9223372036854775807 microseconds 0.1 days';
+SELECT INTERVAL '-9223372036854775808 microseconds -0.1 days';
+
+-- overflowing with fractional fields - ISO8601 format
+SELECT INTERVAL 'P0.1Y2147483647M';
+SELECT INTERVAL 'P-0.1Y-2147483648M';
+SELECT INTERVAL 'P2147483647M0.1Y';
+SELECT INTERVAL 'P-2147483648M-0.1Y';
+
+SELECT INTERVAL 'P0.1M2147483647D';
+SELECT INTERVAL 'P-0.1M-2147483648D';
+SELECT INTERVAL 'P2147483647D0.1M';
+SELECT INTERVAL 'P-2147483648D-0.1M';
+
+SELECT INTERVAL 'P0.5W2147483647D';
+SELECT INTERVAL 'P-0.5W-2147483648D';
+SELECT INTERVAL 'P2147483647D0.5W';
+SELECT INTERVAL 'P-2147483648D-0.5W';
+
+SELECT INTERVAL 'P0.01MT2562047788H54.775807S';
+SELECT INTERVAL 'P-0.01MT-2562047788H-54.775808S';
+
+SELECT INTERVAL 'P0.1DT2562047788H54.775807S';
+SELECT INTERVAL 'P-0.1DT-2562047788H-54.775808S';
+
+SELECT INTERVAL 'PT2562047788.1H54.775807S';
+SELECT INTERVAL 'PT-2562047788.1H-54.775808S';
+
+SELECT INTERVAL 'PT2562047788H0.1M54.775807S';
+SELECT INTERVAL 'PT-2562047788H-0.1M-54.775808S';
+
+-- overflowing with fractional fields - ISO8601 alternative format
+SELECT INTERVAL 'P0.1-2147483647-00';
+SELECT INTERVAL 'P00-0.1-2147483647';
+SELECT INTERVAL 'P00-0.01-00T2562047788:00:54.775807';
+SELECT INTERVAL 'P00-00-0.1T2562047788:00:54.775807';
+SELECT INTERVAL 'PT2562047788.1:00:54.775807';
+SELECT INTERVAL 'PT2562047788:01.:54.775807';
+
+-- overflowing with fractional fields - SQL standard format
+SELECT INTERVAL '0.1 2562047788:0:54.775807';
+SELECT INTERVAL '0.1 2562047788:0:54.775808 ago';
+
+SELECT INTERVAL '2562047788.1:0:54.775807';
+SELECT INTERVAL '2562047788.1:0:54.775808 ago';
+
+SELECT INTERVAL '2562047788:0.1:54.775807';
+SELECT INTERVAL '2562047788:0.1:54.775808 ago';
+
+-- overflowing using AGO with INT_MIN
+SELECT INTERVAL '-2147483648 months ago';
+SELECT INTERVAL '-2147483648 days ago';
+SELECT INTERVAL '-9223372036854775808 microseconds ago';
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 microseconds ago';
+
+-- test that INT_MIN number is formatted properly
+SET IntervalStyle to postgres;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+SET IntervalStyle to postgres_verbose;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+SET IntervalStyle TO sql_standard;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+SET IntervalStyle to iso_8601;
+SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';


^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-02 17:20 ` Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-02 17:20 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

I wrote:
> ... I almost feel that this is
> committable, but there is one thing that is bothering me.  The
> part of DecodeInterval that does strange things with signs in the
> INTSTYLE_SQL_STANDARD case (starting about line 3400 in datetime.c
> before this patch, or line 3600 after) used to separately force the
> hour, minute, second, and microsecond fields to negative.
> Now it forces the merged tm_usec field to negative.  It seems to
> me that this could give a different answer than before, if the
> h/m/s/us values had been of different signs before they got merged.
> However, I don't think that that situation is possible in SQL-spec-
> compliant input, so it may not be a problem.  Again, a counterexample
> would be interesting.

As best I can tell, the case isn't reachable with spec-compliant input,
but it's easy to demonstrate an issue if you set intervalstyle to
sql_standard and then put in Postgres-format input.  Historically,
you got

regression=# show intervalstyle;
 IntervalStyle 
---------------
 postgres
(1 row)

regression=# select '-23 hours 45 min 12.34 sec'::interval;
   interval   
--------------
 -22:14:47.66
(1 row)

(because by default the field signs are taken as independent)

regression=# set intervalstyle = sql_standard ;
SET
regression=# select '-23 hours 45 min 12.34 sec'::interval;
   interval   
--------------
 -23:45:12.34
(1 row)

However, with this patch both cases produce "-22:14:47.66",
because we already merged the differently-signed fields and
DecodeInterval can't tease them apart again.  Perhaps we could
get away with changing this nonstandard corner case, but I'm
pretty uncomfortable with that thought --- I don't think
random semantics changes are within the charter of this patch.

I think the patch can be salvaged, though.  I like the concept
of converting all the sub-day fields to microseconds immediately,
because it avoids a host of issues, so I don't want to give that up.
What I'm going to look into is detecting the sign-adjustment-needed
case up front (which is easy enough, since it's looking at the
input data not the conversion results) and then forcing the
individual field values negative before we accumulate them into
the pg_itm_in struct.

Meanwhile, the fact that we didn't detect this issue immediately
shows that there's a gap in our regression tests.  So the *first*
thing I'm gonna do is push a patch to add test cases like what
I showed above.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-02 17:29   ` Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-02 17:29 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Fri, Apr 1, 2022 at 8:06 PM Tom Lane <[email protected]> wrote:
>
> Joseph Koshakow <[email protected]> writes:
> > * The existing code for rounding had a lot of int to double
> > casting and vice versa. I *think* that doubles are able to completely
> > represent the range of ints. However doubles are not able to represent
> > the full range of int64. After making the change I started noticing
> > a lot of lossy behavior. One thought I had was to change the doubles
> > to long doubles, but I wasn't able to figure out if long doubles could
> > completely represent the range of int64. Especially since their size
> > varies depending on the architecture. Does anyone know the answer to
> > this?
>
> I agree that relying on long double is not a great plan.  However,
> I'm not seeing where there's a problem.  AFAICS the revised code
> only uses doubles to represent fractions from the input, ie if you
> write "123.456 hours" then the ".456" is carried around for awhile
> as a float.  This does not seem likely to pose any real-world
> problem; do you have a counterexample?

Yeah, you're correct, I don't think there is any problem with just
using double. I don't exactly remember why I thought long double
was necessary in the revised code. I probably just confused
myself because it would have been necessary with the old
rounding code, but not the revised code.

> Anyway, I've spent today reviewing the code and cleaning up things
> I didn't like, and attached is a v10.

Thanks so much for the review and updates!

> I think the patch can be salvaged, though.  I like the concept
> of converting all the sub-day fields to microseconds immediately,
> because it avoids a host of issues, so I don't want to give that up.
> What I'm going to look into is detecting the sign-adjustment-needed
> case up front (which is easy enough, since it's looking at the
> input data not the conversion results) and then forcing the
> individual field values negative before we accumulate them into
> the pg_itm_in struct.

This sounds like a very reasonable and achievable approach
to me.

- Joe Koshakow






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-02 18:22     ` Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-02 18:22 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sat, Apr 2, 2022 at 1:29 PM Joseph Koshakow <[email protected]> wrote:
>
> On Fri, Apr 1, 2022 at 8:06 PM Tom Lane <[email protected]> wrote:
> >
> > Joseph Koshakow <[email protected]> writes:
> > > * The existing code for rounding had a lot of int to double
> > > casting and vice versa. I *think* that doubles are able to completely
> > > represent the range of ints. However doubles are not able to represent
> > > the full range of int64. After making the change I started noticing
> > > a lot of lossy behavior. One thought I had was to change the doubles
> > > to long doubles, but I wasn't able to figure out if long doubles could
> > > completely represent the range of int64. Especially since their size
> > > varies depending on the architecture. Does anyone know the answer to
> > > this?
> >
> > I agree that relying on long double is not a great plan.  However,
> > I'm not seeing where there's a problem.  AFAICS the revised code
> > only uses doubles to represent fractions from the input, ie if you
> > write "123.456 hours" then the ".456" is carried around for awhile
> > as a float.  This does not seem likely to pose any real-world
> > problem; do you have a counterexample?
>
> Yeah, you're correct, I don't think there is any problem with just
> using double. I don't exactly remember why I thought long double
> was necessary in the revised code. I probably just confused
> myself because it would have been necessary with the old
> rounding code, but not the revised code.

Ok I actually remember now, the issue is with the rounding
code in AdjustFractMicroseconds.

>    frac *= scale;
>    usec = (int64) frac;
>
>    /* Round off any fractional microsecond */
>    frac -= usec;
>    if (frac > 0.5)
>       usec++;
>    else if (frac < -0.5)
>       usec--;

I believe it's possible for `frac -= usec;` to result in a value greater
than 1 or less than -1 due to the lossiness of int64 to double
conversions. Then we'd incorrectly round in one direction. I don't
have a concrete counter example, but at worst we'd end up with a
result that's a couple of microseconds off, so it's probably not a huge
deal.

If I'm right about the above, and we care enough to fix it, then I think
it can be fixed with the following:

>    frac *= scale;
>    usec = (int64) frac;
>
>    /* Remove non fractional part from frac */
>    frac -= (double) usec;
>    /* Adjust for lossy conversion from int64 to double */
>    while (frac < 0 && frac < -1)
>       frac++;
>    while (frac > 0 && frac > 1)
>       frac--;
>
>    /* Round off any fractional microsecond */
>    if (frac > 0.5)
>       usec++;
>    else if (frac < -0.5)
>       usec--;

- Joe Koshakow






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-02 18:32       ` Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-02 18:32 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sat, Apr 2, 2022 at 2:22 PM Joseph Koshakow <[email protected]> wrote:
>
> On Sat, Apr 2, 2022 at 1:29 PM Joseph Koshakow <[email protected]> wrote:
> >
> > On Fri, Apr 1, 2022 at 8:06 PM Tom Lane <[email protected]> wrote:
> > >
> > > Joseph Koshakow <[email protected]> writes:
> > > > * The existing code for rounding had a lot of int to double
> > > > casting and vice versa. I *think* that doubles are able to completely
> > > > represent the range of ints. However doubles are not able to represent
> > > > the full range of int64. After making the change I started noticing
> > > > a lot of lossy behavior. One thought I had was to change the doubles
> > > > to long doubles, but I wasn't able to figure out if long doubles could
> > > > completely represent the range of int64. Especially since their size
> > > > varies depending on the architecture. Does anyone know the answer to
> > > > this?
> > >
> > > I agree that relying on long double is not a great plan.  However,
> > > I'm not seeing where there's a problem.  AFAICS the revised code
> > > only uses doubles to represent fractions from the input, ie if you
> > > write "123.456 hours" then the ".456" is carried around for awhile
> > > as a float.  This does not seem likely to pose any real-world
> > > problem; do you have a counterexample?
> >
> > Yeah, you're correct, I don't think there is any problem with just
> > using double. I don't exactly remember why I thought long double
> > was necessary in the revised code. I probably just confused
> > myself because it would have been necessary with the old
> > rounding code, but not the revised code.
>
> Ok I actually remember now, the issue is with the rounding
> code in AdjustFractMicroseconds.
>
> >    frac *= scale;
> >    usec = (int64) frac;
> >
> >    /* Round off any fractional microsecond */
> >    frac -= usec;
> >    if (frac > 0.5)
> >       usec++;
> >    else if (frac < -0.5)
> >       usec--;
>
> I believe it's possible for `frac -= usec;` to result in a value greater
> than 1 or less than -1 due to the lossiness of int64 to double
> conversions. Then we'd incorrectly round in one direction. I don't
> have a concrete counter example, but at worst we'd end up with a
> result that's a couple of microseconds off, so it's probably not a huge
> deal.
>
> If I'm right about the above, and we care enough to fix it, then I think
> it can be fixed with the following:
>
> >    frac *= scale;
> >    usec = (int64) frac;
> >
> >    /* Remove non fractional part from frac */
> >    frac -= (double) usec;
> >    /* Adjust for lossy conversion from int64 to double */
> >    while (frac < 0 && frac < -1)
> >       frac++;
> >    while (frac > 0 && frac > 1)
> >       frac--;
> >
> >    /* Round off any fractional microsecond */
> >    if (frac > 0.5)
> >       usec++;
> >    else if (frac < -0.5)
> >       usec--;


Sorry, those should be inclusive comparisons
>    frac *= scale;
>    usec = (int64) frac;
>
>    /* Remove non fractional part from frac */
>    frac -= (double) usec;
>    /* Adjust for lossy conversion from int64 to double */
>    while (frac < 0 && frac <= -1)
>       frac++;
>    while (frac > 0 && frac >= 1)
>       frac--;
>
>    /* Round off any fractional microsecond */
>    if (frac > 0.5)
>       usec++;
>    else if (frac < -0.5)
>       usec--;






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-02 18:55         ` Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-02 18:55 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Fri, Apr 1, 2022 at 8:06 PM Tom Lane <[email protected]> wrote:
> I think the patch can be salvaged, though.  I like the concept
> of converting all the sub-day fields to microseconds immediately,
> because it avoids a host of issues, so I don't want to give that up.
> What I'm going to look into is detecting the sign-adjustment-needed
> case up front (which is easy enough, since it's looking at the
> input data not the conversion results) and then forcing the
> individual field values negative before we accumulate them into
> the pg_itm_in struct.

I took a stab at this issue and the attached patch (which would be
applied on top of your v10 patch) seems to fix the issue. Feel
free to ignore it if you're already working on a fix.

- Joe


Attachments:

  [text/x-patch] 0002-Fix-sql-standard-style-negative-semantics.patch (9.6K, ../../CAAvxfHecFFqpn=5H0T3N1Bki=rkgW0BmRNOZ1NZdKDMCOoy+Rw@mail.gmail.com/2-0002-Fix-sql-standard-style-negative-semantics.patch)
  download | inline diff:
From f43d27142a76fcbabf49e45b9457f8376744e759 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 2 Apr 2022 14:42:18 -0400
Subject: [PATCH 2/2] Fix sql standard style negative semantics

---
 src/backend/utils/adt/datetime.c       | 107 ++++++++++++++-----------
 src/test/regress/expected/interval.out |  14 ++++
 src/test/regress/sql/interval.sql      |   5 ++
 3 files changed, 79 insertions(+), 47 deletions(-)

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index dae90e4a9e..5842d249ab 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -50,6 +50,8 @@ static int	DecodeDate(char *str, int fmask, int *tmask, bool *is2digits,
 static char *AppendSeconds(char *cp, int sec, fsec_t fsec,
 						   int precision, bool fillzeros);
 static bool int64_multiply_add(int64 val, int64 multiplier, int64 *sum);
+static void AdjustForSqlStandardGlobalNegative(int64 *val, double *fval, 
+											   bool global_negative);
 static bool AdjustFractMicroseconds(double frac, int64 scale,
 									struct pg_itm_in *itm_in);
 static bool AdjustFractDays(double frac, int scale,
@@ -527,6 +529,19 @@ int64_multiply_add(int64 val, int64 multiplier, int64 *sum)
 	return true;
 }
 
+/*
+ * Adjust values sign if SQL Standard style is being used and there's a 
+ * single leading negative sign.
+ */
+static void AdjustForSqlStandardGlobalNegative(int64 *val, double *fval,
+											   bool global_negative)
+{
+	if (*val > 0 && global_negative) {
+		*val = -*val;
+		*fval = -*fval;
+	}
+}
+
 /*
  * Multiply frac by scale (to produce microseconds) and add to itm_in->tm_usec.
  * Returns true if successful, false if itm_in overflows.
@@ -3307,10 +3322,43 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 	int64		val;
 	double		fval;
 
+	bool		global_negative = false;
+
 	*dtype = DTK_DELTA;
 	type = IGNORE_DTF;
 	ClearPgItmIn(itm_in);
 
+	/*----------
+	 * The SQL standard defines the interval literal
+	 *	 '-1 1:00:00'
+	 * to mean "negative 1 days and negative 1 hours", while Postgres
+	 * traditionally treats this as meaning "negative 1 days and positive
+	 * 1 hours".  In SQL_STANDARD intervalstyle, we apply the leading sign
+	 * to all fields if there are no other explicit signs.
+	 *
+	 * We leave the signs alone if there are additional explicit signs.
+	 * This protects us against misinterpreting postgres-style dump output,
+	 * since the postgres-style output code has always put an explicit sign on
+	 * all fields following a negative field.  But note that SQL-spec output
+	 * is ambiguous and can be misinterpreted on load!	(So it's best practice
+	 * to dump in postgres style, not SQL style.)
+	 *----------
+	 */
+	if (IntervalStyle == INTSTYLE_SQL_STANDARD && *field[0] == '-')
+	{
+		/* Check for additional explicit signs */
+		bool            more_signs = false;
+		for (i = 1; i < nf; i++)
+		{
+			if (*field[i] == '-' || *field[i] == '+')
+			{
+				more_signs = true;
+				break;
+			}
+		}
+		global_negative = !more_signs;
+	}
+
 	/* read through list backwards to pick up units before values */
 	for (i = nf - 1; i >= 0; i--)
 	{
@@ -3447,18 +3495,21 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 				switch (type)
 				{
 					case DTK_MICROSEC:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustMicroseconds(val, fval, 1, itm_in))
 							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MICROSECOND);
 						break;
 
 					case DTK_MILLISEC:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustMicroseconds(val, fval, 1000, itm_in))
 							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MILLISECOND);
 						break;
 
 					case DTK_SECOND:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustMicroseconds(val, fval, USECS_PER_SEC, itm_in))
 							return DTERR_FIELD_OVERFLOW;
 
@@ -3473,12 +3524,14 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_MINUTE:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustMicroseconds(val, fval, USECS_PER_MINUTE, itm_in))
 							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(MINUTE);
 						break;
 
 					case DTK_HOUR:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustMicroseconds(val, fval, USECS_PER_HOUR, itm_in))
 							return DTERR_FIELD_OVERFLOW;
 						tmask = DTK_M(HOUR);
@@ -3486,6 +3539,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_DAY:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustDays(val, 1, itm_in) ||
 							!AdjustFractMicroseconds(fval, USECS_PER_DAY, itm_in))
 							return DTERR_FIELD_OVERFLOW;
@@ -3493,6 +3547,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_WEEK:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustDays(val, 7, itm_in) ||
 							!AdjustFractDays(fval, 7, itm_in))
 							return DTERR_FIELD_OVERFLOW;
@@ -3500,6 +3555,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_MONTH:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustMonths(val, itm_in) ||
 							!AdjustFractDays(fval, DAYS_PER_MONTH, itm_in))
 							return DTERR_FIELD_OVERFLOW;
@@ -3507,6 +3563,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_YEAR:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustYears(val, 1, itm_in) ||
 							!AdjustFractYears(fval, 1, itm_in))
 							return DTERR_FIELD_OVERFLOW;
@@ -3514,6 +3571,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_DECADE:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustYears(val, 10, itm_in) ||
 							!AdjustFractYears(fval, 10, itm_in))
 							return DTERR_FIELD_OVERFLOW;
@@ -3521,6 +3579,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_CENTURY:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustYears(val, 100, itm_in) ||
 							!AdjustFractYears(fval, 100, itm_in))
 							return DTERR_FIELD_OVERFLOW;
@@ -3528,6 +3587,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 						break;
 
 					case DTK_MILLENNIUM:
+						AdjustForSqlStandardGlobalNegative(&val, &fval, global_negative);
 						if (!AdjustYears(val, 1000, itm_in) ||
 							!AdjustFractYears(fval, 1000, itm_in))
 							return DTERR_FIELD_OVERFLOW;
@@ -3580,53 +3640,6 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 	if (fmask == 0)
 		return DTERR_BAD_FORMAT;
 
-	/*----------
-	 * The SQL standard defines the interval literal
-	 *	 '-1 1:00:00'
-	 * to mean "negative 1 days and negative 1 hours", while Postgres
-	 * traditionally treats this as meaning "negative 1 days and positive
-	 * 1 hours".  In SQL_STANDARD intervalstyle, we apply the leading sign
-	 * to all fields if there are no other explicit signs.
-	 *
-	 * We leave the signs alone if there are additional explicit signs.
-	 * This protects us against misinterpreting postgres-style dump output,
-	 * since the postgres-style output code has always put an explicit sign on
-	 * all fields following a negative field.  But note that SQL-spec output
-	 * is ambiguous and can be misinterpreted on load!	(So it's best practice
-	 * to dump in postgres style, not SQL style.)
-	 *----------
-	 */
-	if (IntervalStyle == INTSTYLE_SQL_STANDARD && *field[0] == '-')
-	{
-		/* Check for additional explicit signs */
-		bool		more_signs = false;
-
-		for (i = 1; i < nf; i++)
-		{
-			if (*field[i] == '-' || *field[i] == '+')
-			{
-				more_signs = true;
-				break;
-			}
-		}
-
-		if (!more_signs)
-		{
-			/*
-			 * Rather than re-determining which field was field[0], just force
-			 * 'em all negative.
-			 */
-			if (itm_in->tm_usec > 0)
-				itm_in->tm_usec = -itm_in->tm_usec;
-			if (itm_in->tm_mday > 0)
-				itm_in->tm_mday = -itm_in->tm_mday;
-			if (itm_in->tm_mon > 0)
-				itm_in->tm_mon = -itm_in->tm_mon;
-			if (itm_in->tm_year > 0)
-				itm_in->tm_year = -itm_in->tm_year;
-		}
-	}
-
 	/* finally, AGO negates everything */
 	if (is_before)
 	{
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 00ffe0e2be..ebcc672d13 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -1690,3 +1690,17 @@ SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
  P-178956970Y-8M-2147483648DT-2562047788H-54.775808S
 (1 row)
 
+SET IntervalStyle to postgres;
+SELECT INTERVAL '-23 hours 45 min 12.34 sec';
+   interval   
+--------------
+ -22:14:47.66
+(1 row)
+
+SET IntervalStyle to sql_standard;
+SELECT INTERVAL '-23 hours 45 min 12.34 sec';
+   interval   
+--------------
+ -23:45:12.34
+(1 row)
+
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index fc924d5bde..bf409df6cb 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -551,3 +551,8 @@ SET IntervalStyle TO sql_standard;
 SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
 SET IntervalStyle to iso_8601;
 SELECT INTERVAL '-2147483648 months -2147483648 days -9223372036854775808 us';
+
+SET IntervalStyle to postgres;
+SELECT INTERVAL '-23 hours 45 min 12.34 sec';
+SET IntervalStyle to sql_standard;
+SELECT INTERVAL '-23 hours 45 min 12.34 sec';
-- 
2.25.1



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-02 19:10           ` Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-02 19:10 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

Joseph Koshakow <[email protected]> writes:
> I took a stab at this issue and the attached patch (which would be
> applied on top of your v10 patch) seems to fix the issue. Feel
> free to ignore it if you're already working on a fix.

You really only need to flip val/fval in one place.  More to the
point, there's also the hh:mm:ss paths to deal with; see my v11.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-02 19:51             ` Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-02 19:51 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sat, Apr 2, 2022 at 3:08 PM Tom Lane <[email protected]> wrote:
>
> Joseph Koshakow <[email protected]> writes:
> > Ok I actually remember now, the issue is with the rounding
> > code in AdjustFractMicroseconds.
> > ...
> > I believe it's possible for `frac -= usec;` to result in a value greater
> > than 1 or less than -1 due to the lossiness of int64 to double
> > conversions.
>
> I think it's not, at least not for the interesting range of possible
> values in this code.  Given that abs(frac) < 1 to start with, the
> abs value of usec can't exceed the value of scale, which is at most
> USECS_PER_DAY so it's at most 37 or so bits, which is well within
> the exact range for any sane implementation of double.  It would
> take a very poor floating-point implementation to not get the right
> answer here.  (And we're largely assuming IEEE-compliant floats these
> days.)

Ah, I see. That makes sense to me.

On Sat, Apr 2, 2022 at 3:10 PM Tom Lane <[email protected]> wrote:
>
> Joseph Koshakow <[email protected]> writes:
> > I took a stab at this issue and the attached patch (which would be
> > applied on top of your v10 patch) seems to fix the issue. Feel
> > free to ignore it if you're already working on a fix.
>
> You really only need to flip val/fval in one place.  More to the
> point, there's also the hh:mm:ss paths to deal with; see my v11.

Good point. Thanks again for all the help!

- Joe Koshakow






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-02 20:14               ` Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-02 20:14 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

Joseph Koshakow <[email protected]> writes:
> On Sat, Apr 2, 2022 at 3:08 PM Tom Lane <[email protected]> wrote:
>> I think it's not, at least not for the interesting range of possible
>> values in this code.  Given that abs(frac) < 1 to start with, the
>> abs value of usec can't exceed the value of scale, which is at most
>> USECS_PER_DAY so it's at most 37 or so bits, which is well within
>> the exact range for any sane implementation of double.  It would
>> take a very poor floating-point implementation to not get the right
>> answer here.  (And we're largely assuming IEEE-compliant floats these
>> days.)

> Ah, I see. That makes sense to me.

Cool.  I've pushed the patch.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-03 07:09                 ` Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-03 07:09 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

I wrote:
> Cool.  I've pushed the patch.

Hmm ... buildfarm's not entirely happy [1][2][3]:

diff -U3 /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/expected/interval.out /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/results/interval.out
--- /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/expected/interval.out	2022-04-03 04:56:32.000000000 +0000
+++ /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/results/interval.out	2022-04-03 05:23:00.000000000 +0000
@@ -1465,7 +1465,7 @@
  LINE 1: select interval 'PT2562047788.1:00:54.775807';
                          ^
  select interval 'PT2562047788:01.:54.775807';
- ERROR:  interval field value out of range: "PT2562047788:01.:54.775807"
+ ERROR:  invalid input syntax for type interval: "PT2562047788:01.:54.775807"
  LINE 1: select interval 'PT2562047788:01.:54.775807';
                          ^
  -- overflowing with fractional fields - SQL standard format

What do you make of that?  I'm betting that strtod() works a
bit differently on those old platforms, but too tired to
look closer tonight.

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer&dt=2022-04-03%2004%3A56%3A34
[2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hoverfly&dt=2022-04-03%2000%3A51%3A50
[3] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=anole&dt=2022-04-03%2000%3A32%3A10






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-03 15:23                   ` Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-03 15:23 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sun, Apr 3, 2022 at 3:09 AM Tom Lane <[email protected]> wrote:
>
> I wrote:
> > Cool.  I've pushed the patch.
>
> Hmm ... buildfarm's not entirely happy [1][2][3]:
>
> diff -U3 /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/expected/interval.out /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/results/interval.out
> --- /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/expected/interval.out 2022-04-03 04:56:32.000000000 +0000
> +++ /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/regress/results/interval.out  2022-04-03 05:23:00.000000000 +0000
> @@ -1465,7 +1465,7 @@
>   LINE 1: select interval 'PT2562047788.1:00:54.775807';
>                           ^
>   select interval 'PT2562047788:01.:54.775807';
> - ERROR:  interval field value out of range: "PT2562047788:01.:54.775807"
> + ERROR:  invalid input syntax for type interval: "PT2562047788:01.:54.775807"
>   LINE 1: select interval 'PT2562047788:01.:54.775807';
>                           ^
>   -- overflowing with fractional fields - SQL standard format
>
> What do you make of that?  I'm betting that strtod() works a
> bit differently on those old platforms, but too tired to
> look closer tonight.
>
>                         regards, tom lane
>
> [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer&dt=2022-04-03%2004%3A56%3A34
> [2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hoverfly&dt=2022-04-03%2000%3A51%3A50
> [3] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=anole&dt=2022-04-03%2000%3A32%3A10

I think I know that the issue is. It's with `ParseISO8601Number` and
the minutes field "1.".
Previously that function parsed the entire field into a single double,
so "1." would
be parsed into 1.0. Now we try to parse the integer and decimal parts
separately. So
we first parse "1" into 1 and then fail to "." into anything because
it's not a valid decimal.

What's interesting is that I believe this syntax, "1.", always would
have failed for
non-ISO8601 Interval. It was only previously valid with ISO8601 intervals.

- Joe Koshakow






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-03 15:44                     ` Tom Lane <[email protected]>
  2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-03 15:44 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

Joseph Koshakow <[email protected]> writes:
> On Sun, Apr 3, 2022 at 3:09 AM Tom Lane <[email protected]> wrote:
>> Hmm ... buildfarm's not entirely happy [1][2][3]:

> I think I know that the issue is. It's with `ParseISO8601Number` and
> the minutes field "1.".
> Previously that function parsed the entire field into a single double,
> so "1." would
> be parsed into 1.0. Now we try to parse the integer and decimal parts
> separately. So
> we first parse "1" into 1 and then fail to "." into anything because
> it's not a valid decimal.

Interesting point, but then why doesn't it fail everywhere?

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-03 16:03                       ` Tom Lane <[email protected]>
  2022-04-03 16:22                         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-03 16:03 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

I wrote:
> Joseph Koshakow <[email protected]> writes:
>> I think I know that the issue is. It's with `ParseISO8601Number` and
>> the minutes field "1.".
>> Previously that function parsed the entire field into a single double,
>> so "1." would
>> be parsed into 1.0. Now we try to parse the integer and decimal parts
>> separately. So
>> we first parse "1" into 1 and then fail to "." into anything because
>> it's not a valid decimal.

> Interesting point, but then why doesn't it fail everywhere?

Oh ... a bit of testing says that strtod() on an empty string
succeeds (returning zero) on Linux, but fails with EINVAL on
AIX.  The latter is a lot less surprising than the former,
so we'd better cope.

(Reading POSIX with an eagle eye, it looks like both behaviors
are allowed per spec: this is why you have to check that endptr
was advanced to be sure everything is kosher.)

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-03 16:22                         ` Joseph Koshakow <[email protected]>
  2022-04-03 16:30                           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-03 16:22 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sun, Apr 3, 2022 at 12:03 PM Tom Lane <[email protected]> wrote:
>
> I wrote:
> > Joseph Koshakow <[email protected]> writes:
> >> I think I know that the issue is. It's with `ParseISO8601Number` and
> >> the minutes field "1.".
> >> Previously that function parsed the entire field into a single double,
> >> so "1." would
> >> be parsed into 1.0. Now we try to parse the integer and decimal parts
> >> separately. So
> >> we first parse "1" into 1 and then fail to "." into anything because
> >> it's not a valid decimal.
>
> > Interesting point, but then why doesn't it fail everywhere?
>
> Oh ... a bit of testing says that strtod() on an empty string
> succeeds (returning zero) on Linux, but fails with EINVAL on
> AIX.  The latter is a lot less surprising than the former,
> so we'd better cope.
>
> (Reading POSIX with an eagle eye, it looks like both behaviors
> are allowed per spec: this is why you have to check that endptr
> was advanced to be sure everything is kosher.)
>
>                         regards, tom lane

I'm not sure I follow exactly. Where would we pass an empty
string to strtod()? Wouldn't we be passing a string with a
single character of '.'?

Either way, from reading the man pages though it seems
that strtod() has the same behavior on any invalid input in
Linux, return 0 and don't advance endptr.

So I think we need to check that endptr has moved both after
the call to strtoi64() and strtod().

- Joe Koshakow






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:22                         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-03 16:30                           ` Tom Lane <[email protected]>
  2022-04-03 16:44                             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-03 16:30 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

Joseph Koshakow <[email protected]> writes:
> On Sun, Apr 3, 2022 at 12:03 PM Tom Lane <[email protected]> wrote:
>> Oh ... a bit of testing says that strtod() on an empty string
>> succeeds (returning zero) on Linux, but fails with EINVAL on
>> AIX.  The latter is a lot less surprising than the former,
>> so we'd better cope.

> I'm not sure I follow exactly. Where would we pass an empty
> string to strtod()? Wouldn't we be passing a string with a
> single character of '.'?

Oh, I was thinking that we passed "cp + 1" to strtod, but that
was just caffeine deprivation.  You're right, what we are asking
it to parse is "." not "".  The result is the same though:
per testing, AIX sets EINVAL and Linux doesn't.

> So I think we need to check that endptr has moved both after
> the call to strtoi64() and strtod().

I'm not sure we need to do that explicitly, given that there's
a check later as to whether endptr is pointing at \0; that will
fail if endptr wasn't advanced.

The fix I was loosely envisioning was to check for cp[1] == '\0'
and not bother calling strtod() in that case.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:22                         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 16:30                           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-03 16:44                             ` Joseph Koshakow <[email protected]>
  2022-04-03 17:00                               ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-03 16:44 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sun, Apr 3, 2022 at 12:30 PM Tom Lane <[email protected]> wrote:
>
> Joseph Koshakow <[email protected]> writes:
> > So I think we need to check that endptr has moved both after
> > the call to strtoi64() and strtod().
>
> I'm not sure we need to do that explicitly, given that there's
> a check later as to whether endptr is pointing at \0; that will
> fail if endptr wasn't advanced.
>
> The fix I was loosely envisioning was to check for cp[1] == '\0'
> and not bother calling strtod() in that case.

Ah, ok I see what you mean. I agree an approach like that should
work, but I don't actually think cp is null terminated in this case. The
entire Interval is passed to DecodeISO8601Interval() as one big
string, so the specific number we're parsing may be somewhere
in the middle.

If we just do the opposite and check isdigit(cp[1]) and only call
strtod() in that case I think it should work.

- Joe Koshakow






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:22                         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 16:30                           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:44                             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-03 17:00                               ` Joseph Koshakow <[email protected]>
  2022-04-03 19:06                                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-03 17:00 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sun, Apr 3, 2022 at 12:44 PM Joseph Koshakow <[email protected]> wrote:
>
> On Sun, Apr 3, 2022 at 12:30 PM Tom Lane <[email protected]> wrote:
> >
> > Joseph Koshakow <[email protected]> writes:
> > > So I think we need to check that endptr has moved both after
> > > the call to strtoi64() and strtod().
> >
> > I'm not sure we need to do that explicitly, given that there's
> > a check later as to whether endptr is pointing at \0; that will
> > fail if endptr wasn't advanced.
> >
> > The fix I was loosely envisioning was to check for cp[1] == '\0'
> > and not bother calling strtod() in that case.
>
> Ah, ok I see what you mean. I agree an approach like that should
> work, but I don't actually think cp is null terminated in this case. The
> entire Interval is passed to DecodeISO8601Interval() as one big
> string, so the specific number we're parsing may be somewhere
> in the middle.
>
> If we just do the opposite and check isdigit(cp[1]) and only call
> strtod() in that case I think it should work.
>
> - Joe Koshakow

How does this patch look? I don't really have any way to test it on
AIX.

- Joe Koshakow


Attachments:

  [text/x-patch] 0001-Fix-parsing-trailing-decimal-point-in-ISO8601.patch (5.1K, ../../CAAvxfHfQG6fe6Xut1GXdYAzH9KyLsdUfJq_QDvLPmPSX_5+pRQ@mail.gmail.com/2-0001-Fix-parsing-trailing-decimal-point-in-ISO8601.patch)
  download | inline diff:
From 46b1ce5a78e21b65536c62ca6270c26c992a1ef7 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sun, 3 Apr 2022 12:58:36 -0400
Subject: [PATCH] Fix parsing trailing decimal point in ISO8601

---
 src/backend/utils/adt/datetime.c       |  9 +++--
 src/test/regress/expected/interval.out | 49 ++++++++++++++++++++++++--
 src/test/regress/sql/interval.sql      | 11 +++++-
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 462f2ed7a8..178313e0d1 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -3676,8 +3676,13 @@ ParseISO8601Number(char *str, char **endptr, int64 *ipart, double *fpart)
 
 	/* Parse fractional part if there is any */
 	if (**endptr == '.')
-		*fpart = strtod(*endptr, endptr) * sign;
-
+	{
+		/* A decimal point with no trailing numbers should be parsed as 0 */
+		if (isdigit((unsigned char) *(*endptr + 1)))
+			*fpart = strtod(*endptr, endptr) * sign;
+		else
+			(*endptr)++;
+	}
 	/* did we not see anything that looks like a number? */
 	if (*endptr == str || errno != 0)
 		return DTERR_BAD_FORMAT;
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 86c8d4bc99..ed051a55c4 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -1464,9 +1464,9 @@ select interval 'PT2562047788.1:00:54.775807';
 ERROR:  interval field value out of range: "PT2562047788.1:00:54.775807"
 LINE 1: select interval 'PT2562047788.1:00:54.775807';
                         ^
-select interval 'PT2562047788:01.:54.775807';
-ERROR:  interval field value out of range: "PT2562047788:01.:54.775807"
-LINE 1: select interval 'PT2562047788:01.:54.775807';
+select interval 'PT2562047788:01:54.775807';
+ERROR:  interval field value out of range: "PT2562047788:01:54.775807"
+LINE 1: select interval 'PT2562047788:01:54.775807';
                         ^
 -- overflowing with fractional fields - SQL standard format
 select interval '0.1 2562047788:0:54.775807';
@@ -1539,6 +1539,49 @@ select interval '-2147483648 months -2147483648 days -9223372036854775808 us';
  @ 178956970 years 8 mons 2147483648 days 2562047788 hours 54.775808 secs ago
 (1 row)
 
+-- check that ISO8601 format accepts trailing '.'
+select interval 'P1.Y2M3DT4H5M6S';
+                   interval                   
+----------------------------------------------
+ @ 1 year 2 mons 3 days 4 hours 5 mins 6 secs
+(1 row)
+
+select interval 'P1Y2.M3DT4H5M6S';
+                   interval                   
+----------------------------------------------
+ @ 1 year 2 mons 3 days 4 hours 5 mins 6 secs
+(1 row)
+
+select interval 'P1Y2M3.DT4H5M6S';
+                   interval                   
+----------------------------------------------
+ @ 1 year 2 mons 3 days 4 hours 5 mins 6 secs
+(1 row)
+
+select interval 'P1Y2M3DT4.H5M6S';
+                   interval                   
+----------------------------------------------
+ @ 1 year 2 mons 3 days 4 hours 5 mins 6 secs
+(1 row)
+
+select interval 'P1Y2M3DT4H5.M6S';
+                   interval                   
+----------------------------------------------
+ @ 1 year 2 mons 3 days 4 hours 5 mins 6 secs
+(1 row)
+
+select interval 'P1Y2M3DT4H5M6.S';
+                   interval                   
+----------------------------------------------
+ @ 1 year 2 mons 3 days 4 hours 5 mins 6 secs
+(1 row)
+
+select interval 'P1.Y2.M3.DT4.H5.M6.S';
+                   interval                   
+----------------------------------------------
+ @ 1 year 2 mons 3 days 4 hours 5 mins 6 secs
+(1 row)
+
 -- check that '30 days' equals '1 month' according to the hash function
 select '30 days'::interval = '1 month'::interval as t;
  t 
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index f05055e03a..62d97f232c 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -474,7 +474,7 @@ select interval 'P00-0.1-2147483647';
 select interval 'P00-0.01-00T2562047788:00:54.775807';
 select interval 'P00-00-0.1T2562047788:00:54.775807';
 select interval 'PT2562047788.1:00:54.775807';
-select interval 'PT2562047788:01.:54.775807';
+select interval 'PT2562047788:01:54.775807';
 
 -- overflowing with fractional fields - SQL standard format
 select interval '0.1 2562047788:0:54.775807';
@@ -502,6 +502,15 @@ select interval '-2147483648 months -2147483648 days -9223372036854775808 us';
 SET IntervalStyle to postgres_verbose;
 select interval '-2147483648 months -2147483648 days -9223372036854775808 us';
 
+-- check that ISO8601 format accepts trailing '.'
+select interval 'P1.Y2M3DT4H5M6S';
+select interval 'P1Y2.M3DT4H5M6S';
+select interval 'P1Y2M3.DT4H5M6S';
+select interval 'P1Y2M3DT4.H5M6S';
+select interval 'P1Y2M3DT4H5.M6S';
+select interval 'P1Y2M3DT4H5M6.S';
+select interval 'P1.Y2.M3.DT4.H5.M6.S';
+
 -- check that '30 days' equals '1 month' according to the hash function
 select '30 days'::interval = '1 month'::interval as t;
 select interval_hash('30 days'::interval) = interval_hash('1 month'::interval) as t;
-- 
2.25.1



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:22                         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 16:30                           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:44                             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 17:00                               ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
@ 2022-04-03 19:06                                 ` Tom Lane <[email protected]>
  2022-04-03 22:19                                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2022-04-03 19:06 UTC (permalink / raw)
  To: Joseph Koshakow <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

Joseph Koshakow <[email protected]> writes:
> How does this patch look? I don't really have any way to test it on
> AIX.

That buildfarm machine is pretty slow, so I'm not in a hurry to test
it manually either.  However, now that we realize the issue is about
whether strtod(".") produces EINVAL or not, I think we need to fix
all the places in datetime.c that are risking that.  After a bit of
hacking I have the attached.  (I think that the call sites for
strtoint and its variants are not at risk of passing empty strings,
so there's not need for concern there.)

BTW, the way you had it coded would allow 'P.Y0M3DT4H5M6S', which
I don't think we want to allow --- at least, that's rejected by v14
on my machine.

			regards, tom lane



Attachments:

  [text/x-diff] 0001-fix-unportable-strtod-calls.patch (6.8K, ../../[email protected]/2-0001-fix-unportable-strtod-calls.patch)
  download | inline diff:
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 462f2ed7a8..4c12c4d663 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -668,19 +668,50 @@ AdjustYears(int64 val, int scale,
 }
 
 
-/* Fetch a fractional-second value with suitable error checking */
+/*
+ * Parse the fractional part of a number (decimal point and optional digits,
+ * followed by end of string).  Returns the fractional value into *frac.
+ *
+ * Returns 0 if successful, DTERR code if bogus input detected.
+ */
+static int
+ParseFraction(char *cp, double *frac)
+{
+	/* Caller should always pass the start of the fraction part */
+	Assert(*cp == '.');
+
+	/*
+	 * We want to allow just "." with no digits, but some versions of strtod
+	 * will report EINVAL for that, so special-case it.
+	 */
+	if (cp[1] == '\0')
+	{
+		*frac = 0;
+	}
+	else
+	{
+		errno = 0;
+		*frac = strtod(cp, &cp);
+		/* check for parse failure */
+		if (*cp != '\0' || errno != 0)
+			return DTERR_BAD_FORMAT;
+	}
+	return 0;
+}
+
+/*
+ * Fetch a fractional-second value with suitable error checking.
+ * Same as ParseFraction except we convert the result to integer microseconds.
+ */
 static int
 ParseFractionalSecond(char *cp, fsec_t *fsec)
 {
 	double		frac;
+	int			dterr;
 
-	/* Caller should always pass the start of the fraction part */
-	Assert(*cp == '.');
-	errno = 0;
-	frac = strtod(cp, &cp);
-	/* check for parse failure */
-	if (*cp != '\0' || errno != 0)
-		return DTERR_BAD_FORMAT;
+	dterr = ParseFraction(cp, &frac);
+	if (dterr)
+		return dterr;
 	*fsec = rint(frac * 1000000);
 	return 0;
 }
@@ -1248,10 +1279,9 @@ DecodeDateTime(char **field, int *ftype, int nf,
 							{
 								double		time;
 
-								errno = 0;
-								time = strtod(cp, &cp);
-								if (*cp != '\0' || errno != 0)
-									return DTERR_BAD_FORMAT;
+								dterr = ParseFraction(cp, &time);
+								if (dterr)
+									return dterr;
 								time *= USECS_PER_DAY;
 								dt2time(time,
 										&tm->tm_hour, &tm->tm_min,
@@ -2146,10 +2176,9 @@ DecodeTimeOnly(char **field, int *ftype, int nf,
 							{
 								double		time;
 
-								errno = 0;
-								time = strtod(cp, &cp);
-								if (*cp != '\0' || errno != 0)
-									return DTERR_BAD_FORMAT;
+								dterr = ParseFraction(cp, &time);
+								if (dterr)
+									return dterr;
 								time *= USECS_PER_DAY;
 								dt2time(time,
 										&tm->tm_hour, &tm->tm_min,
@@ -3035,13 +3064,21 @@ DecodeNumberField(int len, char *str, int fmask,
 		 * Can we use ParseFractionalSecond here?  Not clear whether trailing
 		 * junk should be rejected ...
 		 */
-		double		frac;
+		if (cp[1] == '\0')
+		{
+			/* avoid assuming that strtod will accept "." */
+			*fsec = 0;
+		}
+		else
+		{
+			double		frac;
 
-		errno = 0;
-		frac = strtod(cp, NULL);
-		if (errno != 0)
-			return DTERR_BAD_FORMAT;
-		*fsec = rint(frac * 1000000);
+			errno = 0;
+			frac = strtod(cp, NULL);
+			if (errno != 0)
+				return DTERR_BAD_FORMAT;
+			*fsec = rint(frac * 1000000);
+		}
 		/* Now truncate off the fraction for further processing */
 		*cp = '\0';
 		len = strlen(str);
@@ -3467,11 +3504,9 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 				}
 				else if (*cp == '.')
 				{
-					errno = 0;
-					fval = strtod(cp, &cp);
-					if (*cp != '\0' || errno != 0)
-						return DTERR_BAD_FORMAT;
-
+					dterr = ParseFraction(cp, &fval);
+					if (dterr)
+						return dterr;
 					if (*field[i] == '-')
 						fval = -fval;
 				}
@@ -3650,6 +3685,7 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
  * Helper functions to avoid duplicated code in DecodeISO8601Interval.
  *
  * Parse a decimal value and break it into integer and fractional parts.
+ * Set *endptr to end+1 of the parsed substring.
  * Returns 0 or DTERR code.
  */
 static int
@@ -3676,7 +3712,20 @@ ParseISO8601Number(char *str, char **endptr, int64 *ipart, double *fpart)
 
 	/* Parse fractional part if there is any */
 	if (**endptr == '.')
-		*fpart = strtod(*endptr, endptr) * sign;
+	{
+		/*
+		 * As in ParseFraction, some versions of strtod insist on seeing some
+		 * digits after '.', but some don't.  We want to allow zero digits
+		 * after '.' as long as there were some before it.
+		 */
+		if (isdigit((unsigned char) *(*endptr + 1)))
+			*fpart = strtod(*endptr, endptr) * sign;
+		else
+		{
+			(*endptr)++;		/* advance over '.' */
+			str++;				/* so next test will fail if no digits */
+		}
+	}
 
 	/* did we not see anything that looks like a number? */
 	if (*endptr == str || errno != 0)
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 86c8d4bc99..03f77c01dc 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -908,6 +908,41 @@ select  interval 'P0002'                  AS "year only",
  2 years   | 2 years 10 mons | 2 years 10 mons 15 days | 2 years 00:00:01    | 2 years 10 mons 00:00:01 | 2 years 10 mons 15 days 00:00:01 | 10:00:00  | 10:30:00
 (1 row)
 
+-- Check handling of fractional fields in ISO8601 format.
+select interval 'P1Y0M3DT4H5M6S';
+        interval        
+------------------------
+ 1 year 3 days 04:05:06
+(1 row)
+
+select interval 'P1.0Y0M3DT4H5M6S';
+        interval        
+------------------------
+ 1 year 3 days 04:05:06
+(1 row)
+
+select interval 'P1.1Y0M3DT4H5M6S';
+           interval           
+------------------------------
+ 1 year 1 mon 3 days 04:05:06
+(1 row)
+
+select interval 'P1.Y0M3DT4H5M6S';
+        interval        
+------------------------
+ 1 year 3 days 04:05:06
+(1 row)
+
+select interval 'P.1Y0M3DT4H5M6S';
+       interval        
+-----------------------
+ 1 mon 3 days 04:05:06
+(1 row)
+
+select interval 'P.Y0M3DT4H5M6S';  -- error
+ERROR:  invalid input syntax for type interval: "P.Y0M3DT4H5M6S"
+LINE 1: select interval 'P.Y0M3DT4H5M6S';
+                        ^
 -- test a couple rounding cases that changed since 8.3 w/ HAVE_INT64_TIMESTAMP.
 SET IntervalStyle to postgres_verbose;
 select interval '-10 mons -3 days +03:55:06.70';
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index f05055e03a..97d33a1323 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -312,6 +312,14 @@ select  interval 'P0002'                  AS "year only",
         interval 'PT10'                   AS "hour only",
         interval 'PT10:30'                AS "hour minute";
 
+-- Check handling of fractional fields in ISO8601 format.
+select interval 'P1Y0M3DT4H5M6S';
+select interval 'P1.0Y0M3DT4H5M6S';
+select interval 'P1.1Y0M3DT4H5M6S';
+select interval 'P1.Y0M3DT4H5M6S';
+select interval 'P.1Y0M3DT4H5M6S';
+select interval 'P.Y0M3DT4H5M6S';  -- error
+
 -- test a couple rounding cases that changed since 8.3 w/ HAVE_INT64_TIMESTAMP.
 SET IntervalStyle to postgres_verbose;
 select interval '-10 mons -3 days +03:55:06.70';


^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: Fix overflow in DecodeInterval
  2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:22                         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 16:30                           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
  2022-04-03 16:44                             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 17:00                               ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
  2022-04-03 19:06                                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
@ 2022-04-03 22:19                                   ` Joseph Koshakow <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Joseph Koshakow @ 2022-04-03 22:19 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Sun, Apr 3, 2022 at 3:06 PM Tom Lane <[email protected]> wrote:
> That buildfarm machine is pretty slow, so I'm not in a hurry to test
> it manually either.  However, now that we realize the issue is about
> whether strtod(".") produces EINVAL or not, I think we need to fix
> all the places in datetime.c that are risking that.  After a bit of
> hacking I have the attached.  (I think that the call sites for
> strtoint and its variants are not at risk of passing empty strings,
> so there's not need for concern there.)
>
> BTW, the way you had it coded would allow 'P.Y0M3DT4H5M6S', which
> I don't think we want to allow --- at least, that's rejected by v14
> on my machine.


Oh yeah, good catch. Your patch seems like it should
fix all the issues. Thanks again for the help!

- Joe Koshakow






^ permalink  raw  reply  [nested|flat] 20+ messages in thread


end of thread, other threads:[~2022-04-03 22:19 UTC | newest]

Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-12-20 01:09 [PATCH v29 03/11] Allow to prolong life span of transition tables until transaction end Yugo Nagata <[email protected]>
2022-04-02 00:06 Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-02 17:20 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-02 17:29   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-02 18:22     ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-02 18:32       ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-02 18:55         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-02 19:10           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-02 19:51             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-02 20:14               ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-03 07:09                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-03 15:23                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-03 15:44                     ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-03 16:03                       ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-03 16:22                         ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-03 16:30                           ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-03 16:44                             ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-03 17:00                               ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>
2022-04-03 19:06                                 ` Re: Fix overflow in DecodeInterval Tom Lane <[email protected]>
2022-04-03 22:19                                   ` Re: Fix overflow in DecodeInterval Joseph Koshakow <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox