pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Implement jsonpath .datetime() method
16+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2019-10-21 52ad1e6599 Refactor jsonpath's compareDatetime()
2019-09-25 7881bb14f4 Correctly cast types to Datum and back in compareDatetime()
* pgsql: Implement jsonpath .datetime() method
@ 2019-09-25 19:52 Alexander Korotkov <akorotkov@postgresql.org>
2019-09-25 21:05 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Korotkov @ 2019-09-25 19:52 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Implement jsonpath .datetime() method
This commit implements jsonpath .datetime() method as it's specified in
SQL/JSON standard. There are no-argument and single-argument versions of
this method. No-argument version selects first of ISO datetime formats
matching input string. Single-argument version accepts template string as
its argument.
Additionally to .datetime() method itself this commit also implements
comparison ability of resulting date and time values. There is some difficulty
because exising jsonb_path_*() functions are immutable, while comparison of
timezoned and non-timezoned types involves current timezone. At first, current
timezone could be changes in session. Moreover, timezones themselves are not
immutable and could be updated. This is why we let existing immutable functions
throw errors on such non-immutable comparison. In the same time this commit
provides jsonb_path_*_tz() functions which are stable and support operations
involving timezones. As new functions are added to the system catalog,
catversion is bumped.
Support of .datetime() method was the only blocker prevents T832 from being
marked as supported. sql_features.txt is updated correspondingly.
Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov.
Heavily revised by me. Comments were adjusted by Liudmila Mantrova.
Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com
Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Alexander Korotkov, Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Liudmila Mantrova
Reviewed-by: Anastasia Lubennikova, Peter Eisentraut
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/bffe1bd68457e43925c362d8728ce3b25bdf1c94
Modified Files
--------------
doc/src/sgml/func.sgml | 117 +++++-
src/backend/catalog/sql_features.txt | 2 +-
src/backend/catalog/system_views.sql | 40 +++
src/backend/utils/adt/jsonpath.c | 24 +-
src/backend/utils/adt/jsonpath_exec.c | 470 ++++++++++++++++++++++--
src/backend/utils/adt/jsonpath_gram.y | 14 +
src/backend/utils/adt/jsonpath_scan.l | 1 +
src/backend/utils/errcodes.txt | 1 +
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.dat | 22 ++
src/include/utils/jsonpath.h | 1 +
src/test/regress/expected/jsonb_jsonpath.out | 520 +++++++++++++++++++++++++++
src/test/regress/expected/jsonpath.out | 12 +
src/test/regress/sql/jsonb_jsonpath.sql | 172 +++++++++
src/test/regress/sql/jsonpath.sql | 2 +
15 files changed, 1355 insertions(+), 45 deletions(-)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-25 19:52 pgsql: Implement jsonpath .datetime() method Alexander Korotkov <akorotkov@postgresql.org>
@ 2019-09-25 21:05 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 22:30 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:12 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 16+ messages in thread
From: Alexander Korotkov @ 2019-09-25 21:05 UTC (permalink / raw)
To: Alexander Korotkov <akorotkov@postgresql.org>; +Cc: pgsql-committers <pgsql-committers@lists.postgresql.org>
On Wed, Sep 25, 2019 at 10:52 PM Alexander Korotkov
<akorotkov@postgresql.org> wrote:
> Implement jsonpath .datetime() method
I've noticed buildfarm is unhappy with my commits.
1. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing&dt=2019-09-25%2020%3A40%3A11
2. https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary&dt=2019-09-25%2020%3A38%3A21
I'm investigating the issue.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-25 19:52 pgsql: Implement jsonpath .datetime() method Alexander Korotkov <akorotkov@postgresql.org>
2019-09-25 21:05 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
@ 2019-09-25 22:30 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
1 sibling, 0 replies; 16+ messages in thread
From: Alexander Korotkov @ 2019-09-25 22:30 UTC (permalink / raw)
To: Alexander Korotkov <akorotkov@postgresql.org>; +Cc: pgsql-committers <pgsql-committers@lists.postgresql.org>
On Thu, Sep 26, 2019 at 12:05 AM Alexander Korotkov <
a.korotkov@postgrespro.ru> wrote:
>
> On Wed, Sep 25, 2019 at 10:52 PM Alexander Korotkov
> <akorotkov@postgresql.org> wrote:
> > Implement jsonpath .datetime() method
>
> I've noticed buildfarm is unhappy with my commits.
>
> 1.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing&dt=2019-09-25%2020%3A40%3A11
> 2.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary&dt=2019-09-25%2020%3A38%3A21
Appears to be another issue happening when
USE_FLOAT8_BYVAL is undefined. Managed to reproduce it locally.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-25 19:52 pgsql: Implement jsonpath .datetime() method Alexander Korotkov <akorotkov@postgresql.org>
2019-09-25 21:05 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
@ 2019-09-25 23:12 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-09-25 23:25 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
1 sibling, 1 reply; 16+ messages in thread
From: Tom Lane @ 2019-09-25 23:12 UTC (permalink / raw)
To: Alexander Korotkov <a.korotkov@postgrespro.ru>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> I've noticed buildfarm is unhappy with my commits.
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.
regards, tom lane
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-25 19:52 pgsql: Implement jsonpath .datetime() method Alexander Korotkov <akorotkov@postgresql.org>
2019-09-25 21:05 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:12 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-09-25 23:25 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:56 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Korotkov @ 2019-09-25 23:25 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
On Thu, Sep 26, 2019 at 2:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> > I've noticed buildfarm is unhappy with my commits.
>
> 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.
It seems that caller (executePredicate()) don't NULL values to
compareItems(). It does do only for predicates, which doesn't have
right argument. But binary predicate with lacking argument should be
detected during syntactic analysis.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-25 19:52 pgsql: Implement jsonpath .datetime() method Alexander Korotkov <akorotkov@postgresql.org>
2019-09-25 21:05 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:12 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
2019-09-25 23:25 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
@ 2019-09-25 23:56 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-09-26 00:07 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 1 reply; 16+ messages in thread
From: Tom Lane @ 2019-09-25 23:56 UTC (permalink / raw)
To: Alexander Korotkov <a.korotkov@postgrespro.ru>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> On Thu, Sep 26, 2019 at 2:12 AM Tom Lane <tgl@sss.pgh.pa.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 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.
* 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.
* 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.
* 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.
* OIDs are unsigned, so if you must print them, use %u not %d.
* The errhints don't follow project message style.
* 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.
regards, tom lane
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-25 19:52 pgsql: Implement jsonpath .datetime() method Alexander Korotkov <akorotkov@postgresql.org>
2019-09-25 21:05 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:12 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
2019-09-25 23:25 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:56 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-09-26 00:07 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 0 replies; 16+ messages in thread
From: Alexander Korotkov @ 2019-09-26 00:07 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
On Thu, Sep 26, 2019 at 2:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> > On Thu, Sep 26, 2019 at 2:12 AM Tom Lane <tgl@sss.pgh.pa.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 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.
>
> * 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.
>
> * 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.
>
> * 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.
>
> * OIDs are unsigned, so if you must print them, use %u not %d.
>
> * The errhints don't follow project message style.
>
> * 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.
Thank you for feedback. Nikita and me will provide patches to fix
pointed issues during next couple of days.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
@ 2019-09-27 15:55 Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 1 reply; 16+ messages in thread
From: Nikita Glukhov @ 2019-09-27 15:55 UTC (permalink / raw)
To: pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
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].
> * 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]: https://www.postgresql.org/message-id/d9244568-08bb-5dcf-db25-540412e2e61f%40postgrespro.ru
--
Nikita Glukhov
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
@ 2019-09-29 14:29 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Korotkov @ 2019-09-29 14:29 UTC (permalink / raw)
To: Nikita Glukhov <n.gluhov@postgrespro.ru>; +Cc: pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers <pgsql-hackers@postgresql.org>
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
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
@ 2019-09-30 19:55 ` Robert Haas <robertmhaas@gmail.com>
2019-10-01 17:41 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 1 reply; 16+ messages in thread
From: Robert Haas @ 2019-09-30 19:55 UTC (permalink / raw)
To: Alexander Korotkov <a.korotkov@postgrespro.ru>; +Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>; pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers <pgsql-hackers@postgresql.org>
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
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
@ 2019-10-01 17:41 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-03 13:48 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Korotkov @ 2019-10-01 17:41 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>; pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers <pgsql-hackers@postgresql.org>
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
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
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-01 17:41 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
@ 2019-10-03 13:48 ` Robert Haas <robertmhaas@gmail.com>
2019-10-13 01:52 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 1 reply; 16+ messages in thread
From: Robert Haas @ 2019-10-03 13:48 UTC (permalink / raw)
To: Alexander Korotkov <a.korotkov@postgrespro.ru>; +Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>; pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers <pgsql-hackers@postgresql.org>
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
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-01 17:41 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-03 13:48 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
@ 2019-10-13 01:52 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-13 02:24 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Korotkov @ 2019-10-13 01:52 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>; pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers <pgsql-hackers@postgresql.org>
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.patch (application/octet-stream, 27.4K) download
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
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-01 17:41 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-03 13:48 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-13 01:52 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
@ 2019-10-13 02:24 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-10-14 02:36 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 1 reply; 16+ messages in thread
From: Tom Lane @ 2019-10-13 02:24 UTC (permalink / raw)
To: Alexander Korotkov <a.korotkov@postgrespro.ru>; +Cc: Robert Haas <robertmhaas@gmail.com>; Nikita Glukhov <n.gluhov@postgrespro.ru>; pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; pgsql-hackers <pgsql-hackers@postgresql.org>
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
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-01 17:41 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-03 13:48 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-13 01:52 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-13 02:24 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-10-14 02:36 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-19 16:44 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Korotkov @ 2019-10-14 02:36 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Nikita Glukhov <n.gluhov@postgrespro.ru>; pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; pgsql-hackers <pgsql-hackers@postgresql.org>
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
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: pgsql: Implement jsonpath .datetime() method
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-01 17:41 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-03 13:48 ` Re: pgsql: Implement jsonpath .datetime() method Robert Haas <robertmhaas@gmail.com>
2019-10-13 01:52 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-13 02:24 ` Re: pgsql: Implement jsonpath .datetime() method Tom Lane <tgl@sss.pgh.pa.us>
2019-10-14 02:36 ` Re: pgsql: Implement jsonpath .datetime() method Alexander Korotkov <a.korotkov@postgrespro.ru>
@ 2019-10-19 16:44 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
0 siblings, 0 replies; 16+ messages in thread
From: Alexander Korotkov @ 2019-10-19 16:44 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Nikita Glukhov <n.gluhov@postgrespro.ru>; pgsql-committers <pgsql-committers@lists.postgresql.org>; Alexander Korotkov <akorotkov@postgresql.org>; pgsql-hackers <pgsql-hackers@postgresql.org>
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: download all (2 files, .tar.gz)
0001-Refactor-timestamp2timestamptz_opt_error-3.patch (application/octet-stream, 1.4K) download
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.patch (application/octet-stream, 27.9K) download
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
end of thread, other threads:[~2019-10-19 16:44 UTC | newest]
Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 19:52 pgsql: Implement jsonpath .datetime() method Alexander Korotkov <akorotkov@postgresql.org>
2019-09-25 21:05 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 22:30 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:12 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-09-25 23:25 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-25 23:56 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-09-26 00:07 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-27 15:55 Re: pgsql: Implement jsonpath .datetime() method Nikita Glukhov <n.gluhov@postgrespro.ru>
2019-09-29 14:29 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-09-30 19:55 ` Robert Haas <robertmhaas@gmail.com>
2019-10-01 17:41 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-03 13:48 ` Robert Haas <robertmhaas@gmail.com>
2019-10-13 01:52 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-13 02:24 ` Tom Lane <tgl@sss.pgh.pa.us>
2019-10-14 02:36 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
2019-10-19 16:44 ` Alexander Korotkov <a.korotkov@postgrespro.ru>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox