Re: jsonpath

From: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Robert Haas <robertmhaas(at)gmail(dot)com>, Oleg Bartunov <obartunov(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: jsonpath
Date: 2018-03-27 15:13:11
Message-ID: 880bcbc6-1dba-cf82-7945-c10528f602c4@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Attached 14th version of the patches:
* refactored predicates, introduced 3-valued JsonPathBool type instead of
JsonPathExecResult
* refactored JsonPathExecResult: now it is typedefed to ErrorData *
* fixed recursive wildcard accessor (.**) in strict mode: structural errors
after .** are ignored now

On 20.03.2018 08:36, Tom Lane wrote:

> Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> writes:
>>> That seems like a quite limited list of functions. What about
>>> reworking them providing a way of calling them without risk of
>>> exception?
>> I haven't seen a response to this email. Do we need one before
>> proceeding any further with jsonpath?
> I've not been following this thread in detail, but IMO any code anywhere
> that thinks that no error can be thrown out of non-straight-line code is
> broken beyond redemption. What, for example, happens if we get ENOMEM
> within one of the elog.c functions?

Out of memory are intentionally is not caught here, ereport_safe() is primarily
intended only for handling of errors of ERRCODE_DATA_EXCEPTION category (see
explanation below).

> I did look through 0007-jsonpath-arithmetic-error-handling-v12.patch,
> and I can't believe that's seriously proposed for commit. It's making
> some pretty fragile changes in error handling, and so far as I can
> find there is not even one line of commentary as to what the new
> design rules are supposed to be. Even if it's completely bug-free
> today (which I would bet against), how could we keep it so?
>
> regards, tom lane

I basically agree, this experimental patch is optional (see below).

I think I need to clarify the error handling in SQL/JSON one more time here.

SQL/JSON standard requires us to handle errors being thrown from the JSON path
engine and from SQL/JSON item => SQL type coercions, and then perform the
specified ON ERROR behavior. Standard does not limit handled error categories,
but I think we are interesting here only in errors of category "data exception"
(ERRCODE_DATA_EXCEPTION) in which all possible SQL/JSON, arithmetic, datetime
errors fall. It might be interesting that such error handling has appeared in
standard only with SQL/JSON introduction.

Example of arithmetic error handling in JSON_VALUE:

=# SELECT JSON_VALUE('0', '1 / $' RETURNING int); -- NULL ON ERROR by default
json_value
------------

(1 row)

=# SELECT JSON_VALUE('0', '1 / $' RETURNING int ERROR ON ERROR);
ERROR: division by zero

=# SELECT JSON_VALUE('0', '1 / $' RETURNING int DEFAULT -1 ON ERROR);
json_value
------------
-1
(1 row)

Error handling in SQL/JSON functions is implemented with PG_TRY/PG_CATCH block
with subtransaction (see ExecEvalJsonExpr() in SQL/JSON patch). We had to use
subtransactions here because an arbitrary user-defined typecast function can
be called.

One could think that could only support ERROR ON ERROR behavior and have no
problems with PG_TRY/PG_CATCH, but errors should be handled inside jsonpath
predicates (boolean expression used in filters) regardless of ON ERROR behavior.
Any error in predicate operands is converted directly to special Unknown value
(analogue of SQL NULL).

In the following examples arithmetic errors and errors in conversion of string
into double are handled by comparison predicate itself, not by JSON_QUERY:

=# SELECT JSON_QUERY('[1.23, "4.56", 0, "foo"]',
'$[*] ? (1 / @.double() > 0)' WITH WRAPPER);
json_query
----------------
[1.23, "4.56"]
(1 row)

=# SELECT JSON_QUERY('[1.23, "4.56", 0, "foo"]',
'$[*] ? ((1 / @.double() > 0) is unknown)' WITH WRAPPER);
json_query
------------
[0, "foo"]
(1 row)

Implementation of handling of arithmetic and datetime errors in patch #4 uses
PG_TRY/PG_CATCH without surrounding substransactions, because only the limited
set of built-in functions with known behavior is called inside this block:

numeric_add()
numeric_mul()
numeric_div()
numeric_mod()
numeric_float8()
float8in()
float8_numeric()
to_datetime()

The 7th optional patch is attempt to completely get rid of PG_TRY/PG_CATCH in
jsonpath by the idea of Alexander Korotkov: additional parameter
"ErrorData **edata" is passed to internal variants of those functions for
saving error info into it instead of throwing it. But I agree that this
solution is far from ideal and the assertion that some functions should not
throw exceptions of some category can easily be broken. Moreover, I have done
it only for numeric functions, because to_datetime() function needs too deep
propagation of ErrorData passing.

If your see other variants of implementation of error handling in jsonpath,
please tell us about them. Now we can offer only two variants described above,
but we understand that some future work is necessary to support such standard
features in PostgreSQL without having problems with PG_TRY/PG_CATCH.

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

Attachment Content-Type Size
sqljson_jsonpath_v14.tar.gz application/gzip 76.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-03-27 15:17:17 Re: Parallel safety of binary_upgrade_create_empty_extension
Previous Message Teodor Sigaev 2018-03-27 15:09:17 Re: Cast jsonb to numeric, int, float, bool