Refactor jsonpath's compareDatetime()

This commit refactors come ridiculous coding in compareDatetime().  Also, it
provides correct cross-datatype comparison even when one of values overflows
during cast.  That eliminates dilemma on whether we should suppress overflow
errors during cast.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/32308.1569455803%40sss.pgh.pa.us
Discussion: https://postgr.es/m/a5629d0c-8162-7559-16aa-0c8390d6ba5f%40postgrespro.ru
Author: Nikita Glukhov, Alexander Korotkov

1 files changed, 167 insertions, 95 deletions

diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index a35f718b96e..e2c1bfb5a77 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2298,16 +2298,16 @@ compareItems(int32 op, JsonbValue *jb1, JsonbValue *jb2, bool useTz)
 			break;
 		case jbvDatetime:
 			{
-				bool		have_error = false;
+				bool		cast_error;
 
 				cmp = compareDatetime(jb1->val.datetime.value,
 									  jb1->val.datetime.typid,
 									  jb2->val.datetime.value,
 									  jb2->val.datetime.typid,
 									  useTz,
-									  &have_error);
+									  &cast_error);
 
-				if (have_error)
+				if (cast_error)
 					return jpbUnknown;
 			}
 			break;
@@ -2571,15 +2571,128 @@ wrapItemsInArray(const JsonValueList *items)
 	return pushJsonbValue(&ps, WJB_END_ARRAY, NULL);
 }
 
+/* Check if the timezone required for casting from type1 to type2 is used */
+static void
+checkTimezoneIsUsedForCast(bool useTz, const char *type1, const char *type2)
+{
+	if (!useTz)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot convert value from %s to %s without timezone usage",
+						type1, type2),
+				 errhint("Use *_tz() function for timezone support.")));
+}
+
+/* Convert time datum to timetz datum */
+static Datum
+castTimeToTimeTz(Datum time, bool useTz)
+{
+	checkTimezoneIsUsedForCast(useTz, "time", "timetz");
+
+	return DirectFunctionCall1(time_timetz, time);
+}
+
+/*---
+ * Compares 'ts1' and 'ts2' timestamp, assuming that ts1 might be overflowed
+ * during cast from another datatype.
+ *
+ * 'overflow1' specifies overflow of 'ts1' value:
+ *  0 - no overflow,
+ * -1 - exceed lower boundary,
+ *  1 - exceed upper boundary.
+ */
+static int
+cmpTimestampWithOverflow(Timestamp ts1, int overflow1, Timestamp ts2)
+{
+	/*
+	 * All the timestamps we deal with in jsonpath are produced by
+	 * to_datetime() method.  So, they should be valid.
+	 */
+	Assert(IS_VALID_TIMESTAMP(ts2));
+
+	/*
+	 * Timestamp, which exceed lower (upper) bound, is always lower (higher)
+	 * than any valid timestamp except minus (plus) infinity.
+	 */
+	if (overflow1)
+	{
+		if (overflow1 < 0)
+		{
+			if (TIMESTAMP_IS_NOBEGIN(ts2))
+				return 1;
+			else
+				return -1;
+		}
+		if (overflow1 > 0)
+		{
+			if (TIMESTAMP_IS_NOEND(ts2))
+				return -1;
+			else
+				return 1;
+		}
+	}
+
+	return timestamp_cmp_internal(ts1, ts2);
+}
+
+/*
+ * Compare date to timestamptz without throwing overflow error during cast.
+ */
+static int
+cmpDateToTimestamp(DateADT date1, Timestamp ts2, bool useTz)
+{
+	TimestampTz ts1;
+	int			overflow = 0;
+
+	ts1 = date2timestamp_opt_overflow(date1, &overflow);
+
+	return cmpTimestampWithOverflow(ts1, overflow, ts2);
+}
+
+/*
+ * Compare date to timestamptz without throwing overflow error during cast.
+ */
+static int
+cmpDateToTimestampTz(DateADT date1, TimestampTz tstz2, bool useTz)
+{
+	TimestampTz tstz1;
+	int			overflow = 0;
+
+	checkTimezoneIsUsedForCast(useTz, "date", "timestamptz");
+
+	tstz1 = date2timestamptz_opt_overflow(date1, &overflow);
+
+	return cmpTimestampWithOverflow(tstz1, overflow, tstz2);
+}
+
+/*
+ * Compare timestamp to timestamptz without throwing overflow error during cast.
+ */
+static int
+cmpTimestampToTimestampTz(Timestamp ts1, TimestampTz tstz2, bool useTz)
+{
+	TimestampTz tstz1;
+	int			overflow = 0;
+
+	checkTimezoneIsUsedForCast(useTz, "timestamp", "timestamptz");
+
+	tstz1 = timestamp2timestamptz_opt_overflow(ts1, &overflow);
+
+	return cmpTimestampWithOverflow(tstz1, overflow, tstz2);
+}
+
 /*
  * Cross-type comparison of two datetime SQL/JSON items.  If items are
- * uncomparable, 'error' flag is set.
+ * uncomparable *cast_error flag is set, otherwise *cast_error is unset.
+ * If the cast requires timezone and it is not used, then explicit error is thrown.
  */
 static int
 compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
-				bool useTz, bool *have_error)
+				bool useTz, bool *cast_error)
 {
-	PGFunction cmpfunc = NULL;
+	PGFunction cmpfunc;
+
+	*cast_error = false;
 
 	switch (typid1)
 	{
@@ -2592,31 +2705,23 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 					break;
 
 				case TIMESTAMPOID:
-					val1 = TimestampGetDatum(date2timestamp_opt_error(DatumGetDateADT(val1), have_error));
-					if (have_error && *have_error)
-						return 0;
-					cmpfunc = timestamp_cmp;
-
-					break;
+					return cmpDateToTimestamp(DatumGetDateADT(val1),
+											  DatumGetTimestamp(val2),
+											  useTz);
 
 				case TIMESTAMPTZOID:
-					if (!useTz)
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("cannot convert value from %s to %s without timezone usage",
-										"date", "timestamptz"),
-								 errhint("use *_tz() function for timezone support")));
-					val1 = TimestampTzGetDatum(date2timestamptz_opt_error(DatumGetDateADT(val1), have_error));
-					if (have_error && *have_error)
-						return 0;
-					cmpfunc = timestamp_cmp;
-
-					break;
+					return cmpDateToTimestampTz(DatumGetDateADT(val1),
+												DatumGetTimestampTz(val2),
+												useTz);
 
 				case TIMEOID:
 				case TIMETZOID:
-					*have_error = true;
+					*cast_error = true; /* uncomparable types */
 					return 0;
+
+				default:
+					elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u",
+						 typid2);
 			}
 			break;
 
@@ -2629,13 +2734,7 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 					break;
 
 				case TIMETZOID:
-					if (!useTz)
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("cannot convert value from %s to %s without timezone usage",
-										"time", "timetz"),
-								 errhint("use *_tz() function for timezone support")));
-					val1 = DirectFunctionCall1(time_timetz, val1);
+					val1 = castTimeToTimeTz(val1, useTz);
 					cmpfunc = timetz_cmp;
 
 					break;
@@ -2643,8 +2742,12 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 				case DATEOID:
 				case TIMESTAMPOID:
 				case TIMESTAMPTZOID:
-					*have_error = true;
+					*cast_error = true; /* uncomparable types */
 					return 0;
+
+				default:
+					elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u",
+						 typid2);
 			}
 			break;
 
@@ -2652,13 +2755,7 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 			switch (typid2)
 			{
 				case TIMEOID:
-					if (!useTz)
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("cannot convert value from %s to %s without timezone usage",
-										"time", "timetz"),
-								 errhint("use *_tz() function for timezone support")));
-					val2 = DirectFunctionCall1(time_timetz, val2);
+					val2 = castTimeToTimeTz(val2, useTz);
 					cmpfunc = timetz_cmp;
 
 					break;
@@ -2671,8 +2768,12 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 				case DATEOID:
 				case TIMESTAMPOID:
 				case TIMESTAMPTZOID:
-					*have_error = true;
+					*cast_error = true; /* uncomparable types */
 					return 0;
+
+				default:
+					elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u",
+						 typid2);
 			}
 			break;
 
@@ -2680,12 +2781,9 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 			switch (typid2)
 			{
 				case DATEOID:
-					val2 = TimestampGetDatum(date2timestamp_opt_error(DatumGetDateADT(val2), have_error));
-					if (have_error && *have_error)
-						return 0;
-					cmpfunc = timestamp_cmp;
-
-					break;
+					return -cmpDateToTimestamp(DatumGetDateADT(val2),
+											   DatumGetTimestamp(val1),
+											   useTz);
 
 				case TIMESTAMPOID:
 					cmpfunc = timestamp_cmp;
@@ -2693,23 +2791,18 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 					break;
 
 				case TIMESTAMPTZOID:
-					if (!useTz)
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("cannot convert value from %s to %s without timezone usage",
-										"timestamp", "timestamptz"),
-								 errhint("use *_tz() function for timezone support")));
-					val1 = TimestampTzGetDatum(timestamp2timestamptz_opt_error(DatumGetTimestamp(val1), have_error));
-					if (have_error && *have_error)
-						return 0;
-					cmpfunc = timestamp_cmp;
-
-					break;
+					return cmpTimestampToTimestampTz(DatumGetTimestamp(val1),
+													 DatumGetTimestampTz(val2),
+													 useTz);
 
 				case TIMEOID:
 				case TIMETZOID:
-					*have_error = true;
+					*cast_error = true; /* uncomparable types */
 					return 0;
+
+				default:
+					elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u",
+						 typid2);
 			}
 			break;
 
@@ -2717,32 +2810,14 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 			switch (typid2)
 			{
 				case DATEOID:
-					if (!useTz)
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("cannot convert value from %s to %s without timezone usage",
-										"date", "timestamptz"),
-								 errhint("use *_tz() function for timezone support")));
-					val2 = TimestampTzGetDatum(date2timestamptz_opt_error(DatumGetDateADT(val2), have_error));
-					if (have_error && *have_error)
-						return 0;
-					cmpfunc = timestamp_cmp;
-
-					break;
+					return -cmpDateToTimestampTz(DatumGetDateADT(val2),
+												 DatumGetTimestampTz(val1),
+												 useTz);
 
 				case TIMESTAMPOID:
-					if (!useTz)
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("cannot convert value from %s to %s without timezone usage",
-										"timestamp", "timestamptz"),
-								 errhint("use *_tz() function for timezone support")));
-					val2 = TimestampTzGetDatum(timestamp2timestamptz_opt_error(DatumGetTimestamp(val2), have_error));
-					if (have_error && *have_error)
-						return 0;
-					cmpfunc = timestamp_cmp;
-
-					break;
+					return -cmpTimestampToTimestampTz(DatumGetTimestamp(val2),
+													  DatumGetTimestampTz(val1),
+													  useTz);
 
 				case TIMESTAMPTZOID:
 					cmpfunc = timestamp_cmp;
@@ -2751,24 +2826,21 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 
 				case TIMEOID:
 				case TIMETZOID:
-					*have_error = true;
+					*cast_error = true; /* uncomparable types */
 					return 0;
+
+				default:
+					elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u",
+						 typid2);
 			}
 			break;
 
 		default:
-			elog(ERROR, "unrecognized SQL/JSON datetime type oid: %d",
-				 typid1);
+			elog(ERROR, "unrecognized SQL/JSON datetime type oid: %u", typid1);
 	}
 
-	if (*have_error)
-		return 0;
-
-	if (!cmpfunc)
-		elog(ERROR, "unrecognized SQL/JSON datetime type oid: %d",
-			 typid2);
-
-	*have_error = false;
+	if (*cast_error)
+		return 0;				/* cast error */
 
 	return DatumGetInt32(DirectFunctionCall2(cmpfunc, val1, val2));
 }