Re: pgsql: Implement jsonpath .datetime() method

Started by Nikita Glukhovover 6 years ago9 messages
#1Nikita Glukhov
n.gluhov@postgrespro.ru

On Thu, Sep 26, 2019 at 2:57 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> writes:

On Thu, Sep 26, 2019 at 2:12 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

The proximate problem seems to be that compareItems() is insufficiently
careful to ensure that both values are non-null before passing them
off to datatype-specific code. The code accidentally fails to crash
on 64-bit machines, but it's still giving garbage answers, I think.

I've found compareItems() code to not apply appropriate cast to/from
Datum. Fixed in 7881bb14f4. This makes test pass on my local 32-bit
machine. I'll keep look on buildfarm.

Hm. dromedary seems not to crash either with that fix, but I'm not
sure why not, because when I was running the previous tree by hand,
the stack trace showed pretty clearly that we were getting to
timestamp_cmp with one null and one non-null argument. So I don't
believe your argument that that's impossible, and even if it is,
I do not think it's sane for compareItems to depend on that ---
especially when one of its code paths *does* check for nulls.

I do not have a very good opinion about the quality of this code
upon my first glance at it. Just looking at compareDatetime:

The patch with compareDatetime() refactoring was posted in the original
thread in pgsql-hackers [1]/messages/by-id/d9244568-08bb-5dcf-db25-540412e2e61f@postgrespro.ru.

* The code is schizophrenic about whether it's allowed to pass a
null have_error pointer or not. It is not very sensible to have
some code doing
if (have_error && *have_error)
return 0;
when other code branches will dump core for null have_error.
Given that if this test actually was doing anything, what it
would be doing is failing to detect error conditions, I think
the only sensible design is to insist that have_error mustn't be
null, in which case these checks for null pointer should be removed,
because (a) they waste code and cycles and (b) they mislead the
reader as to what the API of compareDatetime actually is.

* At least some of the code paths will malfunction if the caller
didn't initialize *have_error to false. If that is an intended API
requirement, it is not acceptable for the function header comment
not to say so. (For bonus points, it'd be nice if the header
comment agreed with the code as to the name of the variable.)
If this isn't an intended requirement, you need to fix the code,
probably by initializing "*have_error = false;" up at the top.

* This is silly:

if (*have_error)
return 0;

*have_error = false;

* Also, given that you have that "if (*have_error)" where you do,
the have_error tests inside the switch are useless redundancy.
You might as well just remove them completely and let the final
test handle falling out if a conversion failed. Alternatively
you could drop the final test, because as the code stands right
now, it's visibly impossible to get there with *have_error true.

Yes, oddities in have_error handling seem to appear during numerous
reworks of the patch. have_error is really non-NULL now, and its
handling was simplified in the patch.

* It's a bit schizophrenic also that some of the switches
lack default:s (and rely on the if (!cmpfunc) below), while
the outer switch does have its own, completely redundant
default:. I'd get rid of that default: and instead add
a comment explaining that the !cmpfunc test substitutes for
default branches.

Default cases with elog()s were added to the all switches. Previously,
the default case in the outer switch was used to report invalid type1,
and cmpfunc was used to report invalid type2.

* OIDs are unsigned, so if you must print them, use %u not %d.

Fixed.

* The errhints don't follow project message style.

Fixed, but I'm not sure about "*_tz()". Maybe it's worth to pass current
jsonb_xxx function name to compareDatetime() through JsonPathExecContext?

* The blank lines before "break"s aren't really per project
style either, IMO. They certainly aren't doing anything to
improve readability, and they help limit how much code you
can see at once.

Fixed. If I recall it correctly, these lines were added by pgindent.

* More generally, it's completely unclear why some error conditions
are thrown as errors and others just result in returning *have_error.
In particular, it seems weird that some unsupported datatype combinations
cause hard errors while others do not. Maybe that's fine, but if so,
the function header comment is falling down on the job by not explaining
the reasoning.

All cast errors are caught by jsonpath predicate. Comparison of the
uncomparable datetime types (time[tz] to dated types) also returns Unknown.
And only if datatype conversion requires current timezone, which is not
available in immutable family of jsonb_xxx() functions, hard error is thrown.
This behavior is specific only for our jsonpath implementation. But I'm
really not sure if we should throw an error or return Unknown in this case.

[1]: /messages/by-id/d9244568-08bb-5dcf-db25-540412e2e61f@postgrespro.ru

--
Nikita Glukhov
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

#2Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Nikita Glukhov (#1)

On Fri, Sep 27, 2019 at 6:58 PM Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:

On Thu, Sep 26, 2019 at 2:57 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

* More generally, it's completely unclear why some error conditions
are thrown as errors and others just result in returning *have_error.
In particular, it seems weird that some unsupported datatype combinations
cause hard errors while others do not. Maybe that's fine, but if so,
the function header comment is falling down on the job by not explaining
the reasoning.

All cast errors are caught by jsonpath predicate. Comparison of the
uncomparable datetime types (time[tz] to dated types) also returns Unknown.
And only if datatype conversion requires current timezone, which is not
available in immutable family of jsonb_xxx() functions, hard error is thrown.
This behavior is specific only for our jsonpath implementation. But I'm
really not sure if we should throw an error or return Unknown in this case.

I'd like to share my further thoughts about errors. I think we should
suppress errors defined by standard and which user can expect. So,
user can expect that wrong date format causes an error, division by
zero causes an error and so on. And those errors are defined by
standard.

However, we error is caused by limitation of our implementation, then
suppression doesn't look right to me.

For instance.

# select jsonb_path_query('"1000000-01-01"', '$.datetime() >
"2020-01-01 12:00:00".datetime()'::jsonpath);
jsonb_path_query
------------------
null
(1 row)

# select '1000000-01-01'::date > '2020-01-01 12:00:00'::timestamp;
ERROR: date out of range for timestamp

So, jsonpath behaves like 1000000 is not greater than 2020. This
looks like plain false. And user can't expect that unless she is
familiar with our particular issues. Now I got opinion that such
errors shouldn't be suppressed. We can't suppress *every* error. If
trying to do this, we can come to an idea to suppress OOM error and
return garbage then, which is obviously ridiculous. Opinions?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#3Robert Haas
robertmhaas@gmail.com
In reply to: Alexander Korotkov (#2)

On Sun, Sep 29, 2019 at 10:30 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

So, jsonpath behaves like 1000000 is not greater than 2020. This
looks like plain false. And user can't expect that unless she is
familiar with our particular issues. Now I got opinion that such
errors shouldn't be suppressed. We can't suppress *every* error. If
trying to do this, we can come to an idea to suppress OOM error and
return garbage then, which is obviously ridiculous. Opinions?

I don't know enough about jsonpath to have a view on specifically
which errors ought to be suppressed, but I agree that it's probably
not all of them. In fact, I'd go so far as to say that thinking about
it in terms of error suppression is probably not the right approach in
the first place. Rather, you want to ask what behavior you're trying
to create.

For example, if I'm trying to write a function that takes a string as
input and returns JSON, where the result is formatted as a number if
possible or a string otherwise, I might want access at the C level to
the guts of numeric_in, with all parsing errors returned rather than
thrown. But it would be silly to suppress an out-of-memory condition,
because that doesn't help the caller. The caller wants to know whether
the thing can be parsed as a number or not, and that has nothing to do
with whether we're out of memory, so an out-of-memory error should
still be thrown.

In this case here, it seems to me that you should similarly start by
defining the behavior you're trying to create. Unless that's clearly
defined, deciding which errors to suppress may be difficult.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#4Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Robert Haas (#3)

On Mon, Sep 30, 2019 at 10:56 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Sun, Sep 29, 2019 at 10:30 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

So, jsonpath behaves like 1000000 is not greater than 2020. This
looks like plain false. And user can't expect that unless she is
familiar with our particular issues. Now I got opinion that such
errors shouldn't be suppressed. We can't suppress *every* error. If
trying to do this, we can come to an idea to suppress OOM error and
return garbage then, which is obviously ridiculous. Opinions?

I don't know enough about jsonpath to have a view on specifically
which errors ought to be suppressed, but I agree that it's probably
not all of them. In fact, I'd go so far as to say that thinking about
it in terms of error suppression is probably not the right approach in
the first place. Rather, you want to ask what behavior you're trying
to create.

For example, if I'm trying to write a function that takes a string as
input and returns JSON, where the result is formatted as a number if
possible or a string otherwise, I might want access at the C level to
the guts of numeric_in, with all parsing errors returned rather than
thrown. But it would be silly to suppress an out-of-memory condition,
because that doesn't help the caller. The caller wants to know whether
the thing can be parsed as a number or not, and that has nothing to do
with whether we're out of memory, so an out-of-memory error should
still be thrown.

In this case here, it seems to me that you should similarly start by
defining the behavior you're trying to create. Unless that's clearly
defined, deciding which errors to suppress may be difficult.

Making C functions return errors rather than throw is what we're
implementing in our patchsets. In big picture the behavior we're
trying to create is SQL Standard 2016. It defines error handling as
following.

The SQL operators JSON_VALUE, JSON_QUERY, JSON_TABLE, and JSON_EXISTS provide
the following mechanisms to handle these errors:
1) The SQL/JSON path language traps any errors that occur during the evaluation
of a <JSON filter expression>. Depending on the precise <JSON path predicate>
contained in the <JSON filter expression>, the result may be Unknown, True, or
False, depending on the outcome of non-error tests evaluated in the <JSON path
predicate>.
2) The SQL/JSON path language has two modes, strict and lax, which govern
structural errors, as follows:
a) In lax mode:
i) If an operation requires an SQL/JSON array but the operand is not an SQL
JSON array, then the operand is first “wrapped” in an SQL/JSON array prior
to performing the operation.
ii) If an operation requires something other than an SQL/JSON array, but
the operand is an SQL/JSON array, then the operand is “unwrapped” by
converting its elements into an SQL/JSON sequence prior to performing the
operation.
iii) After applying the preceding resolutions to structural errors, if
there is still a structural error, the result is an empty SQL/JSON
sequence.
b) In strict mode, if the structural error occurs within a <JSON filter
expression>, then the error handling of <JSON filter expression> applies
Otherwise, a structural error is an unhandled error.
3) Non-structural errors outside of a <JSON path predicate> are always
unhandled errors, resulting in an exception condition returned from the path
engine to the SQL/JSON query operator.
4) The SQL/JSON query operators provide an ON ERROR clause to specify the
behavior in case of an input conversion error, an unhandled structural error,
an unhandled non-structural error, or an output conversion error.

So, basically standard requires us to suppress any error happening in
filter expression. But as I wrote before suppression of errors in
datetime comparison may lead to surprising results. That happens in
rare corner cases, but still. This makes uneasy choice between
consistent behavior and standard behavior.

However, Nikita Glukhov gave to good idea about that. Instead on
thinking about whether we should suppress or not cast errors in
datetime comparison, we may just eliminate those error. So, if we
know that casting date to timestamp overflows upper bound of finite
timestamp, then we also know that this date is greater than any finite
timestamp. So, we still able to do correct comparison. I'm going to
implement this and post a patch.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Show quoted text

On Mon, Sep 30, 2019 at 10:56 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Sun, Sep 29, 2019 at 10:30 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

So, jsonpath behaves like 1000000 is not greater than 2020. This
looks like plain false. And user can't expect that unless she is
familiar with our particular issues. Now I got opinion that such
errors shouldn't be suppressed. We can't suppress *every* error. If
trying to do this, we can come to an idea to suppress OOM error and
return garbage then, which is obviously ridiculous. Opinions?

I don't know enough about jsonpath to have a view on specifically
which errors ought to be suppressed, but I agree that it's probably
not all of them. In fact, I'd go so far as to say that thinking about
it in terms of error suppression is probably not the right approach in
the first place. Rather, you want to ask what behavior you're trying
to create.

For example, if I'm trying to write a function that takes a string as
input and returns JSON, where the result is formatted as a number if
possible or a string otherwise, I might want access at the C level to
the guts of numeric_in, with all parsing errors returned rather than
thrown. But it would be silly to suppress an out-of-memory condition,
because that doesn't help the caller. The caller wants to know whether
the thing can be parsed as a number or not, and that has nothing to do
with whether we're out of memory, so an out-of-memory error should
still be thrown.

In this case here, it seems to me that you should similarly start by
defining the behavior you're trying to create. Unless that's clearly
defined, deciding which errors to suppress may be difficult.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#5Robert Haas
robertmhaas@gmail.com
In reply to: Alexander Korotkov (#4)

On Tue, Oct 1, 2019 at 1:41 PM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

So, basically standard requires us to suppress any error happening in
filter expression.

Sounds like the standard is dumb, then. :-)

But as I wrote before suppression of errors in
datetime comparison may lead to surprising results. That happens in
rare corner cases, but still. This makes uneasy choice between
consistent behavior and standard behavior.

Yeah.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#6Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Robert Haas (#5)
1 attachment(s)

On Thu, Oct 3, 2019 at 4:48 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Oct 1, 2019 at 1:41 PM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

So, basically standard requires us to suppress any error happening in
filter expression.

Sounds like the standard is dumb, then. :-)

But as I wrote before suppression of errors in
datetime comparison may lead to surprising results. That happens in
rare corner cases, but still. This makes uneasy choice between
consistent behavior and standard behavior.

Yeah.

Proposed patch eliminates this dilemma in particular case. It
provides correct cross-type comparison of datetime values even if one
of values overflows during cast. In order to do this, I made cast
functions to report whether lower or upper boundary is overflowed. We
know that overflowed value is lower (or upper) than any valid value
except infinity.

This patch also changes the way timestamp to timestamptz cast works.
Previously it did timestamp2tm() then tm2timestamp(). Instead, after
timestamp2tm() it calculates timezone offset and applies it to
original timestamp value. I hope this is correct. If so, besides
making overflow handling easier, this refactoring saves some CPU
cycles.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

0002-Refactor-jsonpath-s-compareDatetime-2.patchapplication/octet-stream; name=0002-Refactor-jsonpath-s-compareDatetime-2.patchDownload
commit a2b2ddcdad2eda14c73b50a97c57e9ba857409da
Author: Alexander Korotkov <akorotkov@postgresql.org>
Date:   Sun Oct 13 04:37:58 2019 +0300

    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

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index fa50d79c05d..709bbaaf5a0 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -554,11 +554,12 @@ date_mii(PG_FUNCTION_ARGS)
 /*
  * Promote date to timestamp.
  *
- * If 'have_error' is NULL, then errors are thrown, else '*have_error' is set
- * and zero is returned.
+ * On overflow error is thrown if 'overflow' is NULL.  Otherwise, '*overflow'
+ * is set to -1 (+1) when result value exceed lower (upper) boundary and zero
+ * returned.
  */
 Timestamp
-date2timestamp_opt_error(DateADT dateVal, bool *have_error)
+date2timestamp_opt_overflow(DateADT dateVal, int *overflow)
 {
 	Timestamp	result;
 
@@ -575,9 +576,9 @@ date2timestamp_opt_error(DateADT dateVal, bool *have_error)
 		 */
 		if (dateVal >= (TIMESTAMP_END_JULIAN - POSTGRES_EPOCH_JDATE))
 		{
-			if (have_error)
+			if (overflow)
 			{
-				*have_error = true;
+				*overflow = 1;
 				return (Timestamp) 0;
 			}
 			else
@@ -596,22 +597,23 @@ date2timestamp_opt_error(DateADT dateVal, bool *have_error)
 }
 
 /*
- * Single-argument version of date2timestamp_opt_error().
+ * Single-argument version of date2timestamp_opt_overflow().
  */
 static TimestampTz
 date2timestamp(DateADT dateVal)
 {
-	return date2timestamp_opt_error(dateVal, NULL);
+	return date2timestamp_opt_overflow(dateVal, NULL);
 }
 
 /*
  * Promote date to timestamp with time zone.
  *
- * If 'have_error' is NULL, then errors are thrown, else '*have_error' is set
- * and zero is returned.
+ * On overflow error is thrown if 'overflow' is NULL.  Otherwise, '*overflow'
+ * is set to -1 (+1) when result value exceed lower (upper) boundary and zero
+ * returned.
  */
 TimestampTz
-date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
+date2timestamptz_opt_overflow(DateADT dateVal, int *overflow)
 {
 	TimestampTz result;
 	struct pg_tm tt,
@@ -631,9 +633,9 @@ date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
 		 */
 		if (dateVal >= (TIMESTAMP_END_JULIAN - POSTGRES_EPOCH_JDATE))
 		{
-			if (have_error)
+			if (overflow)
 			{
-				*have_error = true;
+				*overflow = 1;
 				return (TimestampTz) 0;
 			}
 			else
@@ -659,9 +661,15 @@ date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
 		 */
 		if (!IS_VALID_TIMESTAMP(result))
 		{
-			if (have_error)
+			if (overflow)
 			{
-				*have_error = true;
+				if (result < MIN_TIMESTAMP)
+					*overflow = -1;
+				else
+				{
+					Assert(result >= END_TIMESTAMP);
+					*overflow = 1;
+				}
 				return (TimestampTz) 0;
 			}
 			else
@@ -677,12 +685,12 @@ date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
 }
 
 /*
- * Single-argument version of date2timestamptz_opt_error().
+ * Single-argument version of date2timestamptz_opt_overflow().
  */
 static TimestampTz
 date2timestamptz(DateADT dateVal)
 {
-	return date2timestamptz_opt_error(dateVal, NULL);
+	return date2timestamptz_opt_overflow(dateVal, NULL);
 }
 
 /*
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index a35f718b96e..565ab4a949b 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)
 	{
@@ -2588,35 +2701,26 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 			{
 				case DATEOID:
 					cmpfunc = date_cmp;
-
 					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;
 
@@ -2625,26 +2729,22 @@ compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
 			{
 				case TIMEOID:
 					cmpfunc = time_cmp;
-
 					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;
 
 				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,27 +2752,23 @@ 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;
 
 				case TIMETZOID:
 					cmpfunc = timetz_cmp;
-
 					break;
 
 				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,36 +2776,27 @@ 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;
-
 					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,58 +2804,36 @@ 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;
-
 					break;
 
 				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));
 }
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 84bc97d40c3..1dc4c820de2 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5190,12 +5190,12 @@ timestamp_timestamptz(PG_FUNCTION_ARGS)
 /*
  * Convert timestamp to timestamp with time zone.
  *
- * If 'have_error' is NULL, then errors are thrown, else '*have_error' is set
- * and zero is returned.
+ * On overflow error is thrown if 'overflow' is NULL.  Otherwise, '*overflow'
+ * is set to -1 (+1) when result value exceed lower (upper) boundary and zero
+ * returned.
  */
-
 TimestampTz
-timestamp2timestamptz_opt_error(Timestamp timestamp, bool *have_error)
+timestamp2timestamptz_opt_overflow(Timestamp timestamp, int *overflow)
 {
 	TimestampTz result;
 	struct pg_tm tt,
@@ -5210,27 +5210,39 @@ timestamp2timestamptz_opt_error(Timestamp timestamp, bool *have_error)
 	{
 		tz = DetermineTimeZoneOffset(tm, session_timezone);
 
-		if (!tm2timestamp(tm, fsec, &tz, &result))
+		result = dt2local(timestamp, -tz);
+
+		if (IS_VALID_TIMESTAMP(result))
+		{
 			return result;
+		}
+		else if (overflow)
+		{
+			if (result < MIN_TIMESTAMP)
+				*overflow = -1;
+			else
+			{
+				Assert(result >= END_TIMESTAMP);
+				*overflow = 1;
+			}
+			return (TimestampTz) 0;
+		}
 	}
 
-	if (have_error)
-		*have_error = true;
-	else
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range")));
+	ereport(ERROR,
+			(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+			 errmsg("timestamp out of range")));
 
 	return 0;
 }
 
 /*
- * Single-argument version of timestamp2timestamptz_opt_error().
+ * Single-argument version of timestamp2timestamptz_opt_overflow().
  */
 static TimestampTz
 timestamp2timestamptz(Timestamp timestamp)
 {
-	return timestamp2timestamptz_opt_error(timestamp, NULL);
+	return timestamp2timestamptz_opt_overflow(timestamp, NULL);
 }
 
 /* timestamptz_timestamp()
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index c29f13aaf04..7352b1f4fec 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -70,8 +70,8 @@ typedef struct
 /* date.c */
 extern int32 anytime_typmod_check(bool istz, int32 typmod);
 extern double date2timestamp_no_overflow(DateADT dateVal);
-extern Timestamp date2timestamp_opt_error(DateADT dateVal, bool *have_error);
-extern TimestampTz date2timestamptz_opt_error(DateADT dateVal, bool *have_error);
+extern Timestamp date2timestamp_opt_overflow(DateADT dateVal, int *overflow);
+extern TimestampTz date2timestamptz_opt_overflow(DateADT dateVal, int *overflow);
 extern void EncodeSpecialDate(DateADT dt, char *str);
 extern DateADT GetSQLCurrentDate(void);
 extern TimeTzADT *GetSQLCurrentTime(int32 typmod);
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index e884d4405f5..7652b41eaec 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -97,8 +97,8 @@ extern int	timestamp_cmp_internal(Timestamp dt1, Timestamp dt2);
 /* timestamp comparison works for timestamptz also */
 #define timestamptz_cmp_internal(dt1,dt2)	timestamp_cmp_internal(dt1, dt2)
 
-extern TimestampTz timestamp2timestamptz_opt_error(Timestamp timestamp,
-												   bool *have_error);
+extern TimestampTz timestamp2timestamptz_opt_overflow(Timestamp timestamp,
+													  int *overflow);
 
 extern int	isoweek2j(int year, int week);
 extern void isoweek2date(int woy, int *year, int *mon, int *mday);
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index 063f1c27711..ef8db2d060a 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -1949,17 +1949,17 @@ select jsonb_path_query(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ == "10.03.2017".datetime("dd.mm.yyyy"))');
 ERROR:  cannot convert value from date to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ >= "10.03.2017".datetime("dd.mm.yyyy"))');
 ERROR:  cannot convert value from date to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ <  "10.03.2017".datetime("dd.mm.yyyy"))');
 ERROR:  cannot convert value from date to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ == "10.03.2017".datetime("dd.mm.yyyy"))');
@@ -1996,17 +1996,17 @@ select jsonb_path_query(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ == "12:35".datetime("HH24:MI"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ >= "12:35".datetime("HH24:MI"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ <  "12:35".datetime("HH24:MI"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ == "12:35".datetime("HH24:MI"))');
@@ -2041,17 +2041,17 @@ select jsonb_path_query(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ == "12:35 +1".datetime("HH24:MI TZH"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ >= "12:35 +1".datetime("HH24:MI TZH"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ <  "12:35 +1".datetime("HH24:MI TZH"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ == "12:35 +1".datetime("HH24:MI TZH"))');
@@ -2087,17 +2087,17 @@ select jsonb_path_query(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ >= "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ < "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
@@ -2134,17 +2134,17 @@ select jsonb_path_query(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ >= "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ < "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
@@ -2178,6 +2178,13 @@ select jsonb_path_query_tz(
  "2017-03-10"
 (4 rows)
 
+-- overflow during comparison
+select jsonb_path_query('"1000000-01-01"', '$.datetime() > "2020-01-01 12:00:00".datetime()'::jsonpath);
+ jsonb_path_query 
+------------------
+ true
+(1 row)
+
 set time zone default;
 -- jsonpath operators
 SELECT jsonb_path_query('[{"a": 1}, {"a": 2}]', '$[*]');
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index 246e38b9edd..591be00278f 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -516,6 +516,9 @@ select jsonb_path_query_tz(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ < "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 
+-- overflow during comparison
+select jsonb_path_query('"1000000-01-01"', '$.datetime() > "2020-01-01 12:00:00".datetime()'::jsonpath);
+
 set time zone default;
 
 -- jsonpath operators
#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alexander Korotkov (#6)

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:

This patch also changes the way timestamp to timestamptz cast works.
Previously it did timestamp2tm() then tm2timestamp(). Instead, after
timestamp2tm() it calculates timezone offset and applies it to
original timestamp value. I hope this is correct.

I'd wonder whether this gives the same answers near DST transitions,
where it's not real clear which offset applies.

Please *don't* wrap this sort of thing into an unrelated feature patch.

regards, tom lane

#8Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Tom Lane (#7)

On Sun, Oct 13, 2019 at 5:24 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:

This patch also changes the way timestamp to timestamptz cast works.
Previously it did timestamp2tm() then tm2timestamp(). Instead, after
timestamp2tm() it calculates timezone offset and applies it to
original timestamp value. I hope this is correct.

I'd wonder whether this gives the same answers near DST transitions,
where it's not real clear which offset applies.

I will try this and share the results.

Please *don't* wrap this sort of thing into an unrelated feature patch.

Sure, thank you for noticing.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#9Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Alexander Korotkov (#8)
2 attachment(s)

On Mon, Oct 14, 2019 at 5:36 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

On Sun, Oct 13, 2019 at 5:24 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:

This patch also changes the way timestamp to timestamptz cast works.
Previously it did timestamp2tm() then tm2timestamp(). Instead, after
timestamp2tm() it calculates timezone offset and applies it to
original timestamp value. I hope this is correct.

I'd wonder whether this gives the same answers near DST transitions,
where it's not real clear which offset applies.

I will try this and share the results.

I've separated refactoring of timestamp to timestamptz cast into a
separate patch. Patchset is attached.

I've investigates the behavior near DST transitions in Moscow
timezone. Last two DST transitions it had in 2010-03-28 and
2010-10-31. It behaves the same with and without patch. The tests
are below.

# set timezone = 'Europe/Moscow';

# select '2010-03-28 01:59:59'::timestamp::timestamptz;
timestamptz
------------------------
2010-03-28 01:59:59+03
(1 row)

# select '2010-03-28 02:00:00'::timestamp::timestamptz;
timestamptz
------------------------
2010-03-28 03:00:00+04
(1 row)

# select '2010-03-28 02:59:59'::timestamp::timestamptz;
timestamptz
------------------------
2010-03-28 03:59:59+04
(1 row)

# select '2010-03-28 03:00:00'::timestamp::timestamptz;
timestamptz
------------------------
2010-03-28 03:00:00+04
(1 row)

# select '2010-10-31 01:59:59'::timestamp::timestamptz;
timestamptz
------------------------
2010-10-31 01:59:59+04
(1 row)

# select '2010-10-31 02:00:00'::timestamp::timestamptz;
timestamptz
------------------------
2010-10-31 02:00:00+03
(1 row)

BTW, I've noticed how ridiculous cast behaves for values in the range
of [2010-03-28 02:00:00, 2010-03-28 03:00:00). Now, I think that
timestamptz type, which explicitly stores timezone offset, has some
point. At least, it would be possible to save the same local time
value during casts.

I'm going to push these two patches if no objections.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

0001-Refactor-timestamp2timestamptz_opt_error-3.patchapplication/octet-stream; name=0001-Refactor-timestamp2timestamptz_opt_error-3.patchDownload
From 2e215c0ddb3665c8145f22520bce905efe6d954d Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sat, 19 Oct 2019 11:18:32 +0300
Subject: [PATCH 1/2] Refactor timestamp2timestamptz_opt_error()

While casting from timestamp to timestamptz we do timestamp2tm() then
tm2timestamp().  This commit eliminates call to tm2timestamp().  Instead, it
directly applies timezone offset to the original timestamp value.  This make
upcoming jsonpath overflow handling easier.  Also, it should save us some CPU
cycles.

Discussion: https://postgr.es/m/CAPpHfdvRPRh_mTGar5WmDeRZ%3DU5dOXHdxspYYD%3D76m3knNGjXA%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Tom Lane
---
 src/backend/utils/adt/timestamp.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 84bc97d40c3..90ebb50e1f5 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5210,8 +5210,17 @@ timestamp2timestamptz_opt_error(Timestamp timestamp, bool *have_error)
 	{
 		tz = DetermineTimeZoneOffset(tm, session_timezone);
 
-		if (!tm2timestamp(tm, fsec, &tz, &result))
+		result = dt2local(timestamp, -tz);
+
+		if (IS_VALID_TIMESTAMP(result))
+		{
 			return result;
+		}
+		else if (have_error)
+		{
+			*have_error = true;
+			return (TimestampTz) 0;
+		}
 	}
 
 	if (have_error)
-- 
2.14.3

0002-Refactor-jsonpath-s-compareDatetime-3.patchapplication/octet-stream; name=0002-Refactor-jsonpath-s-compareDatetime-3.patchDownload
From 862235cb97556b65295b68462def820488c28ec3 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sun, 13 Oct 2019 04:37:58 +0300
Subject: [PATCH 2/2] 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
---
 src/backend/utils/adt/date.c                 |  40 ++--
 src/backend/utils/adt/jsonpath_exec.c        | 262 +++++++++++++++++----------
 src/backend/utils/adt/timestamp.c            |  31 ++--
 src/include/utils/date.h                     |   4 +-
 src/include/utils/timestamp.h                |   4 +-
 src/test/regress/expected/jsonb_jsonpath.out |  37 ++--
 src/test/regress/sql/jsonb_jsonpath.sql      |   3 +
 7 files changed, 237 insertions(+), 144 deletions(-)

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index fa50d79c05d..709bbaaf5a0 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -554,11 +554,12 @@ date_mii(PG_FUNCTION_ARGS)
 /*
  * Promote date to timestamp.
  *
- * If 'have_error' is NULL, then errors are thrown, else '*have_error' is set
- * and zero is returned.
+ * On overflow error is thrown if 'overflow' is NULL.  Otherwise, '*overflow'
+ * is set to -1 (+1) when result value exceed lower (upper) boundary and zero
+ * returned.
  */
 Timestamp
-date2timestamp_opt_error(DateADT dateVal, bool *have_error)
+date2timestamp_opt_overflow(DateADT dateVal, int *overflow)
 {
 	Timestamp	result;
 
@@ -575,9 +576,9 @@ date2timestamp_opt_error(DateADT dateVal, bool *have_error)
 		 */
 		if (dateVal >= (TIMESTAMP_END_JULIAN - POSTGRES_EPOCH_JDATE))
 		{
-			if (have_error)
+			if (overflow)
 			{
-				*have_error = true;
+				*overflow = 1;
 				return (Timestamp) 0;
 			}
 			else
@@ -596,22 +597,23 @@ date2timestamp_opt_error(DateADT dateVal, bool *have_error)
 }
 
 /*
- * Single-argument version of date2timestamp_opt_error().
+ * Single-argument version of date2timestamp_opt_overflow().
  */
 static TimestampTz
 date2timestamp(DateADT dateVal)
 {
-	return date2timestamp_opt_error(dateVal, NULL);
+	return date2timestamp_opt_overflow(dateVal, NULL);
 }
 
 /*
  * Promote date to timestamp with time zone.
  *
- * If 'have_error' is NULL, then errors are thrown, else '*have_error' is set
- * and zero is returned.
+ * On overflow error is thrown if 'overflow' is NULL.  Otherwise, '*overflow'
+ * is set to -1 (+1) when result value exceed lower (upper) boundary and zero
+ * returned.
  */
 TimestampTz
-date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
+date2timestamptz_opt_overflow(DateADT dateVal, int *overflow)
 {
 	TimestampTz result;
 	struct pg_tm tt,
@@ -631,9 +633,9 @@ date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
 		 */
 		if (dateVal >= (TIMESTAMP_END_JULIAN - POSTGRES_EPOCH_JDATE))
 		{
-			if (have_error)
+			if (overflow)
 			{
-				*have_error = true;
+				*overflow = 1;
 				return (TimestampTz) 0;
 			}
 			else
@@ -659,9 +661,15 @@ date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
 		 */
 		if (!IS_VALID_TIMESTAMP(result))
 		{
-			if (have_error)
+			if (overflow)
 			{
-				*have_error = true;
+				if (result < MIN_TIMESTAMP)
+					*overflow = -1;
+				else
+				{
+					Assert(result >= END_TIMESTAMP);
+					*overflow = 1;
+				}
 				return (TimestampTz) 0;
 			}
 			else
@@ -677,12 +685,12 @@ date2timestamptz_opt_error(DateADT dateVal, bool *have_error)
 }
 
 /*
- * Single-argument version of date2timestamptz_opt_error().
+ * Single-argument version of date2timestamptz_opt_overflow().
  */
 static TimestampTz
 date2timestamptz(DateADT dateVal)
 {
-	return date2timestamptz_opt_error(dateVal, NULL);
+	return date2timestamptz_opt_overflow(dateVal, NULL);
 }
 
 /*
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));
 }
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 90ebb50e1f5..1dc4c820de2 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5190,12 +5190,12 @@ timestamp_timestamptz(PG_FUNCTION_ARGS)
 /*
  * Convert timestamp to timestamp with time zone.
  *
- * If 'have_error' is NULL, then errors are thrown, else '*have_error' is set
- * and zero is returned.
+ * On overflow error is thrown if 'overflow' is NULL.  Otherwise, '*overflow'
+ * is set to -1 (+1) when result value exceed lower (upper) boundary and zero
+ * returned.
  */
-
 TimestampTz
-timestamp2timestamptz_opt_error(Timestamp timestamp, bool *have_error)
+timestamp2timestamptz_opt_overflow(Timestamp timestamp, int *overflow)
 {
 	TimestampTz result;
 	struct pg_tm tt,
@@ -5216,30 +5216,33 @@ timestamp2timestamptz_opt_error(Timestamp timestamp, bool *have_error)
 		{
 			return result;
 		}
-		else if (have_error)
+		else if (overflow)
 		{
-			*have_error = true;
+			if (result < MIN_TIMESTAMP)
+				*overflow = -1;
+			else
+			{
+				Assert(result >= END_TIMESTAMP);
+				*overflow = 1;
+			}
 			return (TimestampTz) 0;
 		}
 	}
 
-	if (have_error)
-		*have_error = true;
-	else
-		ereport(ERROR,
-				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
-				 errmsg("timestamp out of range")));
+	ereport(ERROR,
+			(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+			 errmsg("timestamp out of range")));
 
 	return 0;
 }
 
 /*
- * Single-argument version of timestamp2timestamptz_opt_error().
+ * Single-argument version of timestamp2timestamptz_opt_overflow().
  */
 static TimestampTz
 timestamp2timestamptz(Timestamp timestamp)
 {
-	return timestamp2timestamptz_opt_error(timestamp, NULL);
+	return timestamp2timestamptz_opt_overflow(timestamp, NULL);
 }
 
 /* timestamptz_timestamp()
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index c29f13aaf04..7352b1f4fec 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -70,8 +70,8 @@ typedef struct
 /* date.c */
 extern int32 anytime_typmod_check(bool istz, int32 typmod);
 extern double date2timestamp_no_overflow(DateADT dateVal);
-extern Timestamp date2timestamp_opt_error(DateADT dateVal, bool *have_error);
-extern TimestampTz date2timestamptz_opt_error(DateADT dateVal, bool *have_error);
+extern Timestamp date2timestamp_opt_overflow(DateADT dateVal, int *overflow);
+extern TimestampTz date2timestamptz_opt_overflow(DateADT dateVal, int *overflow);
 extern void EncodeSpecialDate(DateADT dt, char *str);
 extern DateADT GetSQLCurrentDate(void);
 extern TimeTzADT *GetSQLCurrentTime(int32 typmod);
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index e884d4405f5..7652b41eaec 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -97,8 +97,8 @@ extern int	timestamp_cmp_internal(Timestamp dt1, Timestamp dt2);
 /* timestamp comparison works for timestamptz also */
 #define timestamptz_cmp_internal(dt1,dt2)	timestamp_cmp_internal(dt1, dt2)
 
-extern TimestampTz timestamp2timestamptz_opt_error(Timestamp timestamp,
-												   bool *have_error);
+extern TimestampTz timestamp2timestamptz_opt_overflow(Timestamp timestamp,
+													  int *overflow);
 
 extern int	isoweek2j(int year, int week);
 extern void isoweek2date(int woy, int *year, int *mon, int *mday);
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index 063f1c27711..ef8db2d060a 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -1949,17 +1949,17 @@ select jsonb_path_query(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ == "10.03.2017".datetime("dd.mm.yyyy"))');
 ERROR:  cannot convert value from date to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ >= "10.03.2017".datetime("dd.mm.yyyy"))');
 ERROR:  cannot convert value from date to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ <  "10.03.2017".datetime("dd.mm.yyyy"))');
 ERROR:  cannot convert value from date to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["2017-03-10", "2017-03-11", "2017-03-09", "12:34:56", "01:02:03 +04", "2017-03-10 00:00:00", "2017-03-10 12:34:56", "2017-03-10 01:02:03 +04", "2017-03-10 03:00:00 +03"]',
 	'$[*].datetime() ? (@ == "10.03.2017".datetime("dd.mm.yyyy"))');
@@ -1996,17 +1996,17 @@ select jsonb_path_query(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ == "12:35".datetime("HH24:MI"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ >= "12:35".datetime("HH24:MI"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ <  "12:35".datetime("HH24:MI"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["12:34:00", "12:35:00", "12:36:00", "12:35:00 +00", "12:35:00 +01", "13:35:00 +01", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +01"]',
 	'$[*].datetime() ? (@ == "12:35".datetime("HH24:MI"))');
@@ -2041,17 +2041,17 @@ select jsonb_path_query(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ == "12:35 +1".datetime("HH24:MI TZH"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ >= "12:35 +1".datetime("HH24:MI TZH"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ <  "12:35 +1".datetime("HH24:MI TZH"))');
 ERROR:  cannot convert value from time to timetz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["12:34:00 +01", "12:35:00 +01", "12:36:00 +01", "12:35:00 +02", "12:35:00 -02", "10:35:00", "11:35:00", "12:35:00", "2017-03-10", "2017-03-10 12:35:00", "2017-03-10 12:35:00 +1"]',
 	'$[*].datetime() ? (@ == "12:35 +1".datetime("HH24:MI TZH"))');
@@ -2087,17 +2087,17 @@ select jsonb_path_query(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ >= "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ < "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["2017-03-10 12:34:00", "2017-03-10 12:35:00", "2017-03-10 12:36:00", "2017-03-10 12:35:00 +01", "2017-03-10 13:35:00 +01", "2017-03-10 12:35:00 -01", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35".datetime("dd.mm.yyyy HH24:MI"))');
@@ -2134,17 +2134,17 @@ select jsonb_path_query(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ >= "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ < "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 ERROR:  cannot convert value from timestamp to timestamptz without timezone usage
-HINT:  use *_tz() function for timezone support
+HINT:  Use *_tz() function for timezone support.
 select jsonb_path_query_tz(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ == "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
@@ -2178,6 +2178,13 @@ select jsonb_path_query_tz(
  "2017-03-10"
 (4 rows)
 
+-- overflow during comparison
+select jsonb_path_query('"1000000-01-01"', '$.datetime() > "2020-01-01 12:00:00".datetime()'::jsonpath);
+ jsonb_path_query 
+------------------
+ true
+(1 row)
+
 set time zone default;
 -- jsonpath operators
 SELECT jsonb_path_query('[{"a": 1}, {"a": 2}]', '$[*]');
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index 246e38b9edd..591be00278f 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -516,6 +516,9 @@ select jsonb_path_query_tz(
 	'["2017-03-10 12:34:00 +01", "2017-03-10 12:35:00 +01", "2017-03-10 12:36:00 +01", "2017-03-10 12:35:00 +02", "2017-03-10 12:35:00 -02", "2017-03-10 10:35:00", "2017-03-10 11:35:00", "2017-03-10 12:35:00", "2017-03-10", "2017-03-11", "12:34:56", "12:34:56 +01"]',
 	'$[*].datetime() ? (@ < "10.03.2017 12:35 +1".datetime("dd.mm.yyyy HH24:MI TZH"))');
 
+-- overflow during comparison
+select jsonb_path_query('"1000000-01-01"', '$.datetime() > "2020-01-01 12:00:00".datetime()'::jsonpath);
+
 set time zone default;
 
 -- jsonpath operators
-- 
2.14.3