From d6d5149d7395ee454eea3e0f630cb56556ae4681 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Thu, 21 Sep 2023 15:34:33 +0530
Subject: [PATCH v27 5/8] Use integer overflow checking routines to add and
 subtract finite intervals

Use pg_add/sub_s32/64_overflow() routines in finite_interval_pl() and
finite_interval_mi() instead of adding overflow checks in those
functions.

Ashutosh Bapat
---
 src/backend/utils/adt/timestamp.c      | 121 +++++++++++--------------
 src/test/regress/expected/interval.out |  11 +++
 src/test/regress/expected/window.out   |  54 ++++-------
 src/test/regress/sql/interval.sql      |   3 +
 src/test/regress/sql/window.sql        |  22 ++---
 5 files changed, 95 insertions(+), 116 deletions(-)

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 450b547036..4b35ee0bdf 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -99,6 +99,8 @@ static Timestamp timestamptz2timestamp(TimestampTz timestamp);
 static void interval_um_internal(Interval *interval, Interval *result);
 static void finite_interval_pl(Interval *result, Interval *span1,
 							   Interval *span2);
+static void finite_interval_mi(Interval *result, Interval *span1,
+							   Interval *span2);
 
 /* common code for timestamptypmodin and timestamptztypmodin */
 static int32
@@ -3502,34 +3504,7 @@ interval_mi(PG_FUNCTION_ARGS)
 	else if (INTERVAL_IS_NOEND(span2))
 		INTERVAL_NOBEGIN(result);
 	else
-	{
-		result->month = span1->month - span2->month;
-		/* overflow check copied from int4mi */
-		if (!SAMESIGN(span1->month, span2->month) &&
-			!SAMESIGN(result->month, span1->month))
-			ereport(ERROR,
-					(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-					 errmsg("interval out of range")));
-
-		result->day = span1->day - span2->day;
-		if (!SAMESIGN(span1->day, span2->day) &&
-			!SAMESIGN(result->day, span1->day))
-			ereport(ERROR,
-					(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-					 errmsg("interval out of range")));
-
-		result->time = span1->time - span2->time;
-		if (!SAMESIGN(span1->time, span2->time) &&
-			!SAMESIGN(result->time, span1->time))
-			ereport(ERROR,
-					(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-					 errmsg("interval out of range")));
-
-		if (INTERVAL_NOT_FINITE(result))
-			ereport(ERROR,
-					(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-					 errmsg("interval out of range")));
-	}
+		finite_interval_mi(result, span1, span2);
 
 	PG_RETURN_INTERVAL_P(result);
 }
@@ -3869,11 +3844,18 @@ makeIntervalAggState(FunctionCallInfo fcinfo)
 }
 
 /*
- * Function to add two finite intervals.
+ * Functions to add or subtract two finite intervals.
+ *
+ * We handle non-finite intervals in different ways when accumulating/discarding
+ * intervals and in actual mathematical operations respectively. But the
+ * addition or subtraction of finite intervals have same implementation
+ * respectively in both these cases.
+ *
+ * Addition/Subtraction of span1 and span2 is stored in the result. It is fine
+ * to pass either of span1 or span2 as a result pointer. In such a case result
+ * will overwrite the given value.
  *
- * We handle non-finite intervals in different ways when accumulating intervals
- * and adding two intervals respectively. But the addition of finite interval
- * has same implementation in both these cases.
+ * If the result is not a valid interval, the function throws an error.
  */
 static void
 finite_interval_pl(Interval *result, Interval *span1, Interval *span2)
@@ -3881,24 +3863,44 @@ finite_interval_pl(Interval *result, Interval *span1, Interval *span2)
 	Assert(!INTERVAL_NOT_FINITE(span1));
 	Assert(!INTERVAL_NOT_FINITE(span2));
 
-	result->month = span1->month + span2->month;
-	/* overflow check copied from int4pl */
-	if (SAMESIGN(span1->month, span2->month) &&
-		!SAMESIGN(result->month, span1->month))
+	if (pg_add_s32_overflow(span1->month, span2->month, &result->month))
+		ereport(ERROR,
+				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+				 errmsg("interval out of range")));
+
+	if (pg_add_s32_overflow(span1->day, span2->day, &result->day))
+		ereport(ERROR,
+				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+				 errmsg("interval out of range")));
+
+	if (pg_add_s64_overflow(span1->time, span2->time, &result->time))
+		ereport(ERROR,
+				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+				 errmsg("interval out of range")));
+
+	if (INTERVAL_NOT_FINITE(result))
+		ereport(ERROR,
+				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+				 errmsg("interval out of range")));
+}
+
+static void
+finite_interval_mi(Interval *result, Interval *span1, Interval *span2)
+{
+	Assert(!INTERVAL_NOT_FINITE(span1));
+	Assert(!INTERVAL_NOT_FINITE(span2));
+
+	if (pg_sub_s32_overflow(span1->month, span2->month, &result->month))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("interval out of range")));
 
-	result->day = span1->day + span2->day;
-	if (SAMESIGN(span1->day, span2->day) &&
-		!SAMESIGN(result->day, span1->day))
+	if (pg_sub_s32_overflow(span1->day, span2->day, &result->day))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("interval out of range")));
 
-	result->time = span1->time + span2->time;
-	if (SAMESIGN(span1->time, span2->time) &&
-		!SAMESIGN(result->time, span1->time))
+	if (pg_sub_s64_overflow(span1->time, span2->time, &result->time))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
 				 errmsg("interval out of range")));
@@ -3915,9 +3917,6 @@ finite_interval_pl(Interval *result, Interval *span1, Interval *span2)
 static void
 do_interval_accum(IntervalAggState *state, Interval *newval)
 {
-
-	Interval	temp;
-
 	/* Count -infinity inputs separately from all else */
 	if (INTERVAL_IS_NOBEGIN(newval))
 	{
@@ -3932,8 +3931,7 @@ do_interval_accum(IntervalAggState *state, Interval *newval)
 		return;
 	}
 
-	memcpy(&temp, &state->sumX, sizeof(Interval));
-	finite_interval_pl(&state->sumX, &temp, newval);
+	finite_interval_pl(&state->sumX, &state->sumX, newval);
 	state->N++;
 }
 
@@ -3959,6 +3957,9 @@ interval_avg_accum(PG_FUNCTION_ARGS)
 
 /*
  * Combine function for interval aggregates.
+ *
+ * Combine the given internal aggregate states and place the combination in the
+ * first argument.
  */
 Datum
 interval_avg_combine(PG_FUNCTION_ARGS)
@@ -3972,9 +3973,9 @@ interval_avg_combine(PG_FUNCTION_ARGS)
 	if (state2 == NULL)
 		PG_RETURN_POINTER(state1);
 
-	/* manually copy all fields from state2 to state1 */
 	if (state1 == NULL)
 	{
+		/* manually copy all fields from state2 to state1 */
 		state1 = makeIntervalAggState(fcinfo);
 
 		state1->N = state2->N;
@@ -3992,14 +3993,9 @@ interval_avg_combine(PG_FUNCTION_ARGS)
 	state1->pInfcount += state2->pInfcount;
 	state1->nInfcount += state2->nInfcount;
 
+	/* Accumulate finite interval values, if any. */
 	if (state2->N > 0)
-	{
-		Interval	temp;
-
-		/* Accumulate interval values */
-		memcpy(&temp, &state1->sumX, sizeof(Interval));
-		finite_interval_pl(&state1->sumX, &temp, &state2->sumX);
-	}
+		finite_interval_pl(&state1->sumX, &state1->sumX, &state2->sumX);
 
 	PG_RETURN_POINTER(state1);
 }
@@ -4025,24 +4021,15 @@ do_interval_discard(IntervalAggState *state, Interval *newval)
 	}
 
 	/* Handle to be discarded finite value. */
+	state->N--;
 	if (state->N > 0)
-	{
-		Interval	temp;
-		Interval	neg_val;
-
-		neg_val.day = -newval->day;
-		neg_val.month = -newval->month;
-		neg_val.time = -newval->time;
-
-		memcpy(&temp, &state->sumX, sizeof(Interval));
-		finite_interval_pl(&state->sumX, &temp, &neg_val);
-	}
+		finite_interval_mi(&state->sumX, &state->sumX, newval);
 	else
 	{
 		/* All values discarded, reset the state */
+		Assert(state->N == 0);
 		memset(&state->sumX, 0, sizeof(state->sumX));
 	}
-	state->N--;
 }
 
 /*
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 135d683285..8bd72f60b0 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -347,6 +347,17 @@ SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1;
 (5 rows)
 
 RESET enable_seqscan;
+-- subtracting about-to-overflow values should result in 0
+SELECT f1 - f1 FROM INTERVAL_TBL_OF;
+ ?column? 
+----------
+ 00:00:00
+ 00:00:00
+ 00:00:00
+ 00:00:00
+ 00:00:00
+(5 rows)
+
 DROP TABLE INTERVAL_TBL_OF;
 -- Test multiplication and division with intervals.
 -- Floating point arithmetic rounding errors can lead to unexpected results,
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 45262924cd..63f36e069e 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -4375,51 +4375,37 @@ SELECT i,AVG(v::interval) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND UNBOUNDE
  4 | 
 (4 rows)
 
---order by.
-SELECT  x
-        ,avg(x) OVER(ORDER BY x ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_avg
-        ,sum(x) OVER(ORDER BY x ROWS BETWEEN 1 PRECEDING AND CURRENT ROW ) as prev1_curr_sum
-FROM (VALUES (NULL::interval),
-               ('infinity'::interval),
-               ('infinity'::timestamptz - now()),
-               ('6 days'::interval),
-               (NULL::interval),
-               ('-infinity'::interval)) v(x);
-     x     | curr_next_avg | prev1_curr_sum 
------------+---------------+----------------
- -infinity | -infinity     | -infinity
- @ 6 days  | infinity      | -infinity
- infinity  | infinity      | infinity
- infinity  | infinity      | infinity
-           |               | infinity
-           |               | 
-(6 rows)
-
---no order by.
+-- window function over interval, infinity and extreme values test the
+-- behaviour of accumulation and elimination of these values as the window
+-- slides.
 SELECT  x
         ,avg(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_avg
-        ,sum(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_sum
         ,avg(x) OVER(ROWS BETWEEN 1 PRECEDING AND CURRENT ROW ) as prev1_curr_avg
+        ,sum(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_sum
         ,sum(x) OVER(ROWS BETWEEN 1 PRECEDING AND CURRENT ROW ) as prev1_curr_sum
-        ,avg(x) OVER(ROWS BETWEEN 1 PRECEDING AND 2 FOLLOWING ) as prev1_next2_avg
-        ,sum(x) OVER(ROWS BETWEEN 1 PRECEDING AND 2 FOLLOWING ) as prev1_next2_sum
 FROM (VALUES (NULL::interval),
                ('infinity'::interval),
+               ('2147483647 days 2147483646 months'), -- extreme interval value
                ('infinity'::timestamptz - now()),
+               ('-2147483648 days -2147483647 months'), -- extreme interval value
+               ('infinity'::interval),
                ('6 days'::interval),
                ('7 days'::interval),
                (NULL::interval),
                ('-infinity'::interval)) v(x);
-     x     |   curr_next_avg   | curr_next_sum |  prev1_curr_avg   | prev1_curr_sum | prev1_next2_avg | prev1_next2_sum 
------------+-------------------+---------------+-------------------+----------------+-----------------+-----------------
-           | infinity          | infinity      |                   |                | infinity        | infinity
- infinity  | infinity          | infinity      | infinity          | infinity       | infinity        | infinity
- infinity  | infinity          | infinity      | infinity          | infinity       | infinity        | infinity
- @ 6 days  | @ 6 days 12 hours | @ 13 days     | infinity          | infinity       | infinity        | infinity
- @ 7 days  | @ 7 days          | @ 7 days      | @ 6 days 12 hours | @ 13 days      | -infinity       | -infinity
-           | -infinity         | -infinity     | @ 7 days          | @ 7 days       | -infinity       | -infinity
- -infinity | -infinity         | -infinity     | -infinity         | -infinity      | -infinity       | -infinity
-(7 rows)
+                      x                       |   curr_next_avg   |  prev1_curr_avg   | curr_next_sum | prev1_curr_sum 
+----------------------------------------------+-------------------+-------------------+---------------+----------------
+                                              | infinity          |                   | infinity      | 
+ infinity                                     | infinity          | infinity          | infinity      | infinity
+ @ 178956970 years 6 mons 2147483647 days     | infinity          | infinity          | infinity      | infinity
+ infinity                                     | infinity          | infinity          | infinity      | infinity
+ @ 178956970 years 7 mons 2147483648 days ago | infinity          | infinity          | infinity      | infinity
+ infinity                                     | infinity          | infinity          | infinity      | infinity
+ @ 6 days                                     | @ 6 days 12 hours | infinity          | @ 13 days     | infinity
+ @ 7 days                                     | @ 7 days          | @ 6 days 12 hours | @ 7 days      | @ 13 days
+                                              | -infinity         | @ 7 days          | -infinity     | @ 7 days
+ -infinity                                    | -infinity         | -infinity         | -infinity     | -infinity
+(10 rows)
 
 --should fail.
 SELECT x, avg(x) OVER(ROWS BETWEEN CURRENT ROW AND 2 FOLLOWING)
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 912803a1b0..414c12be1c 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -99,6 +99,9 @@ SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1;
 SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1;
 RESET enable_seqscan;
 
+-- subtracting about-to-overflow values should result in 0
+SELECT f1 - f1 FROM INTERVAL_TBL_OF;
+
 DROP TABLE INTERVAL_TBL_OF;
 
 -- Test multiplication and division with intervals.
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index d00423946d..6dd9885949 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -1591,28 +1591,20 @@ SELECT i,AVG(v::numeric) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND UNBOUNDED
 SELECT i,AVG(v::interval) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING)
   FROM (VALUES(1,'1 sec'),(2,'2 sec'),(3,NULL),(4,NULL)) t(i,v);
 
---order by.
-SELECT  x
-        ,avg(x) OVER(ORDER BY x ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_avg
-        ,sum(x) OVER(ORDER BY x ROWS BETWEEN 1 PRECEDING AND CURRENT ROW ) as prev1_curr_sum
-FROM (VALUES (NULL::interval),
-               ('infinity'::interval),
-               ('infinity'::timestamptz - now()),
-               ('6 days'::interval),
-               (NULL::interval),
-               ('-infinity'::interval)) v(x);
-
---no order by.
+-- window function over interval, infinity and extreme values test the
+-- behaviour of accumulation and elimination of these values as the window
+-- slides.
 SELECT  x
         ,avg(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_avg
-        ,sum(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_sum
         ,avg(x) OVER(ROWS BETWEEN 1 PRECEDING AND CURRENT ROW ) as prev1_curr_avg
+        ,sum(x) OVER(ROWS BETWEEN CURRENT ROW AND 1 FOLLOWING ) as curr_next_sum
         ,sum(x) OVER(ROWS BETWEEN 1 PRECEDING AND CURRENT ROW ) as prev1_curr_sum
-        ,avg(x) OVER(ROWS BETWEEN 1 PRECEDING AND 2 FOLLOWING ) as prev1_next2_avg
-        ,sum(x) OVER(ROWS BETWEEN 1 PRECEDING AND 2 FOLLOWING ) as prev1_next2_sum
 FROM (VALUES (NULL::interval),
                ('infinity'::interval),
+               ('2147483647 days 2147483646 months'), -- extreme interval value
                ('infinity'::timestamptz - now()),
+               ('-2147483648 days -2147483647 months'), -- extreme interval value
+               ('infinity'::interval),
                ('6 days'::interval),
                ('7 days'::interval),
                (NULL::interval),
-- 
2.35.3

