MIME-Version: 1.0
References: <a5629d0c-8162-7559-16aa-0c8390d6ba5f@postgrespro.ru>
In-Reply-To: <a5629d0c-8162-7559-16aa-0c8390d6ba5f@postgrespro.ru>
From: Alexander Korotkov <a.korotkov@postgrespro.ru>
Date: Sun, 29 Sep 2019 17:29:56 +0300
Message-ID: 
 <CAPpHfdtOS270Jf7qNZwVg-aAMLgW5PJmfXS5FsB48ekxvHp3Sw@mail.gmail.com>
Subject: Re: pgsql: Implement jsonpath .datetime() method
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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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