From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
To: Nikita Glukhov <n.gluhov@postgrespro.ru>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Stas Kelvich <s.kelvich@postgrespro.ru>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: David Steele <david@pgmasters.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Oleg Bartunov <obartunov@gmail.com>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Subject: Re: jsonpath
Date: Fri, 18 Jan 2019 23:32:48 +0100
Message-ID: <dea8e62f-001d-671f-e3ad-8a7f9cf318d0@2ndquadrant.com> (raw)
In-Reply-To: <CAPpHfdsA09V3s5Q6gtCgYMmyQPOFAj7=Ez1bb7Q0eriFjJugkQ@mail.gmail.com>
References: <CAA8=A796mY7kJdKARt7B+faW70-c8mRh+Ff_joBJ+NyCFLv3Gg@mail.gmail.com>
<CAEepm=0jJ4fkEWLHc2Rqt5F4Und6qd+60pxFQmCnbf0nYYjrcg@mail.gmail.com>
<3bc15ece-5219-e4ca-0b5c-fcd2b276aa26@postgrespro.ru>
<4e2aaa88-3bbf-6af3-8dad-d5687fbec89f@postgrespro.ru>
<42272F2B-16D1-43D6-B22F-273AE1DA6D6B@postgrespro.ru>
<715c1c44-9bb9-6b9f-7c59-61b66578f107@postgrespro.ru>
<20181002023317.GI11712@paquier.xyz>
<063560d8-bc0b-945b-8796-5ba998d79dc6@2ndquadrant.com>
<9a175c2d-b0a9-475f-078b-febc185282ff@postgrespro.ru>
<86a0e7df-443b-3f20-70b0-655a1f5dcbb2@2ndquadrant.com>
<2265b7fe-3b0e-c437-fe15-ac1807319b73@2ndquadrant.com>
<64e3108c-888f-092e-30a2-58dea157707e@postgrespro.ru>
<43e52ed7-9c21-db2c-693a-9245f9e8ce2a@2ndquadrant.com>
<38018365-5793-2827-e3c1-5a3b259fb6ae@postgrespro.ru>
<CAPpHfdvVyXsD1gSwVB7A2C07Mx_6XZOOpEBqO_Le11mYJyq8YQ@mail.gmail.com>
<885de241-5a51-29c8-a6b3-f1dda22aba13@postgrespro.ru>
<CAPpHfdsA09V3s5Q6gtCgYMmyQPOFAj7=Ez1bb7Q0eriFjJugkQ@mail.gmail.com>
On 1/18/19 6:58 PM, Alexander Korotkov wrote:
> Nikita, thank you!
>
> I noticed another thing. 3-arguments version of functions
> jsonpath_exists(), jsonpath_predicate(), jsonpath_query(),
> jsonpath_query_wrapped() are:
>
> 1) Not documented
> 2) Not used in operator definitions
> 3) Not used in subsequent patches
>
> So, it looks like we can just remove then. But I think we're very
> likely can commit jsonpath patch to PostgreSQL 12, but I'm not sure
> about other patches. So, having those functions exposed to user can
> be extremely useful until we have separate nodes for SQL/JSON. So, I
> vote to document these functions.
>
That seems a bit strange. If those functions are meant to be used by
other patches (which ones?) then why should not we make them part of
those future patches?
But it seems more like those functions are actually meant to be used by
users in the first place, in cases when we need to provide a third
parameter (which operators can't do). In that case yes - we should have
them documented properly, but also tested. Which is not the case
currently, because the regression tests only use the operators.
Two more comments:
1) I'm wondering why we even need separate functions for the different
numbers of arguments at the C level, as both simply call to the same
function anyway with a PG_NARGS() condition in it. Can't we ditch those
wrappers and reference that target function directly?
2) I once again ran into the jsonb->json business, which essentially
means that when you do this:
select json '{"a": { "b" : 10}}' @? '$.a.b';
it ends up calling jsonb_jsonpath_exists(), which then does this:
Jsonb *jb = PG_GETARG_JSONB_P(0);
I and am not sure why/how it seems to work, but I find it confusing
because the stack still looks like this:
#0 jsonb_jsonpath_exists (fcinfo=0x162f800) at jsonpath_exec.c:2857
#1 0x000000000096d721 in json_jsonpath_exists2 (fcinfo=0x162f800) at
jsonpath_exec.c:2882
#2 0x00000000006c790a in ExecInterpExpr (state=0x162f300,
econtext=0x162ee18, isnull=0x7ffcea4c3857) at execExprInterp.c:648
...
and of course, the fcinfo->flinfo still points to the json-specific
function, which say the first parameter type is 'json'.
(gdb) print *fcinfo->flinfo
$23 = {fn_addr = 0x96d709 <json_jsonpath_exists2>, fn_oid = 6043,
fn_nargs = 2, fn_strict = true, fn_retset = false, fn_stats = 2 '\002',
fn_extra = 0x0, fn_mcxt = 0x162e990, fn_expr = 0x15db378}
test=# select proname, prosrc, proargtypes from pg_proc where oid = 6043;
proname | prosrc | proargtypes
-----------------+-----------------------+-------------
jsonpath_exists | json_jsonpath_exists2 | 114 6050
(1 row)
test=# select typname from pg_type where oid = 114;
typname
---------
json
(1 row)
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, a.korotkov@postgrespro.ru, n.gluhov@postgrespro.ru, michael@paquier.xyz, s.kelvich@postgrespro.ru, thomas.munro@enterprisedb.com, david@pgmasters.net, tgl@sss.pgh.pa.us, andrew.dunstan@2ndquadrant.com, robertmhaas@gmail.com, obartunov@gmail.com, pavel.stehule@gmail.com
Subject: Re: jsonpath
In-Reply-To: <dea8e62f-001d-671f-e3ad-8a7f9cf318d0@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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