|From:||Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru>|
|To:||Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>|
|Cc:||PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, David Steele <david(at)pgmasters(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, 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>|
|Views:||Raw Message | Whole Thread | Download mbox|
> On 23 Aug 2018, at 00:29, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
> Attached 17th version of the patches rebased onto the current master.
> Nothing significant has changed.
I’ve looked through presented implementation of jsonpath and have some remarks:
1. Current patch set adds functions named jsonpath_* that implementing subset of functionality
of corresponding json_* functions which doesn’t require changes to postgres SQL grammar.
If we are going to commit json_* functions it will be quite strange for end user: two sets of functions
with mostly identical behaviour, except that one of them allows customise error handling and pass variable via
special language constructions. I suggest only leave operators in this patch, but remove functions.
2. Handling of Unknown value differs from one that described in tech report: filter '(@.k == 1)’
doesn’t evaluates to Unknown. Consider following example:
select '42'::json @* '$ ? ( (@.k == 1) is unknown )';
As per my understanding of standard this should show 42. Seems that it was evaluated to False instead,
because boolean operators (! || &&) on filter expression with structural error behave like it is False.
3. .keyvalue() returns object with field named ‘key’ instead of ’name’ as per tech report. ‘key’ field seems to
be more consistent with function name, but i’m not sure it is worths of mismatch with standard. Also ‘id’ field
is omitted, making it harder to use something like GROUP BY afterwards.
4. Looks like jsonpath executor lacks some CHECK_FOR_INTERRUPTS() during hot paths. Backend with
following query is unresponsive to signals:
select count(*) from (
select '[0,1,2,3,4,5,6,7,8,9]'::json @* longpath::jsonpath from (
select '$[' || string_agg(subscripts, ',') ||']' as longpath from (
select 'last,1' as subscripts from generate_series(1,1000000)
5. Files generated by lex/bison should be listed in .gitignore files in corresponding directories.
6. My compiler complains about unused functions: JsonValueListConcat, JsonValueListClear.
7. Presented patch files structure is somewhat complicated with patches to patches. I've melded them
down to following patches:
0001: three first patches with preliminary datetime infrastructure
0002: Jsonpath engine and operators that is your previous 4+6+7
0003: Jsonpath extensions is your previous 8+9
0004: GIN support is your 5th path
Also this patches were formed with 'git format-patch', so one can apply all of them with 'git apply'
restoring each one as commit.
8. Also sending jsonpath_check.sql with queries which I used to check compliance with the standard.
That can be added to regression test, if you want.
|Next Message||Dean Rasheed||2018-09-04 14:16:28||Re: [HACKERS] PATCH: multivariate histograms and MCV lists|
|Previous Message||Tom Lane||2018-09-04 14:04:03||Re: [HACKERS] git down|