0002-Fix-sql-standard-style-negative-semantics.patch

From f43d27142a76fcbabf49e45b9457f8376744e759 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <koshy44@gmail.com>
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