Re: jsonpath

From: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
To: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Cc: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru>, 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>, Robert Haas <robertmhaas(at)gmail(dot)com>, Oleg Bartunov <obartunov(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Subject: Re: jsonpath
Date: 2019-01-11 22:21:13
Message-ID: 885de241-5a51-29c8-a6b3-f1dda22aba13@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Attached 23rd version of the patches.

On 05.01.2019 3:11, Alexander Korotkov wrote:
> On Tue, Dec 4, 2018 at 2:23 AM Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>> 2) We define both DCH_FF# and DCH_ff#, but we never ever use the
>> lower-case version. Heck, it's not mentioned even in DCH_keywords, which
>> does this:
>>
>> ...
>> {"FF1", 3, DCH_FF1, false, FROM_CHAR_DATE_NONE}, /* F */
>> ...
>> {"ff1", 3, DCH_FF1, false, FROM_CHAR_DATE_NONE}, /* F */
>> ...
>>
>> Compare that to DCH_DAY, DCH_Day and DCH_day, mapped to "DAY", "Day" and
>> "day".
>>
>> Yes, "ff#" are mapped to DCH_FF# like "mi" is mapped DCH_MI.
>>
>> "Day", "day" are not mapped to DCH_DAY because they determine letter case in the
>> output, but "ff1" and "FF#" output contains only digits.
> Right, DCH_poz is also offset in DCH_keywords array. So, if array has
> an entry for "ff1" then enum should have a DCH_ff1 member in the same
> position.
>
> I got some other questions regarding this patchset.
>
> 1) Why do we parse FF7-FF9 if we're not supporting them anyway?
> Without defining FF7-FF9 we can also don't throw errors for them
> everywhere. That would save us some code lines.
FF7-FF9 were removed.
> 2) + DCH_to_char_fsec("%01d", in->fsec / INT64CONST(100000));
> Why do we use INT64CONST() here and in the similar places assuming
> that fsec is only uint32?
Fixed.

> 3) wrapItem() is unused in
> 0002-Jsonpath-engine-and-operators-v21.patch, but used in
> 0006-Jsonpath-syntax-extensions-v21.patch. Please, move it to
> 0006-Jsonpath-syntax-extensions-v21.patch?
wraptItem() was moved into patch #6.

> 4) I also got these couple of warning during compilation.
>
> jsonpath_exec.c:1485:1: warning: unused function
> 'recursiveExecuteNested' [-Wunused-function]
> recursiveExecuteNested(JsonPathExecContext *cxt, JsonPathItem *jsp,
> ^
> 1 warning generated.
> jsonpath_scan.l:444:6: warning: implicit declaration of function
> 'jsonpath_yyparse' is invalid in C99 [-Wimplicit-function-declaration]
> if (jsonpath_yyparse((void*)&parseresult) != 0)
> ^
> 1 warning generated.
>
> Perhaps recursiveExecuteNested() is unsed in this patchset. It's
> probably used by some subsequent SQL/JSON-related patchset. So,
> please, move it there.

recursiveExecuteNested() was removed from this patch set.

Prototype for jsonpath_yyparse() should be in the Bison-generated file
src/include/adt/jsonpath_gram.h.

> 5) I think each usage of PG_TRY()/PG_CATCH() deserves comment
> describing why it's safe to use without subtransaction. In fact we
> should just state that no function called inside performs data
> modification.
Comments were added.

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

Attachment Content-Type Size
0001-Preliminary-datetime-infrastructure-v23.patch text/x-patch 31.5 KB
0002-Jsonpath-engine-and-operators-v23.patch text/x-patch 318.9 KB
0003-Remove-PG_TRY-in-jsonpath-arithmetics-v23.patch text/x-patch 28.3 KB
0004-Jsonpath-docs-v23.patch text/x-patch 40.1 KB
0005-Jsonpath-GIN-support-v23.patch text/x-patch 47.0 KB
0006-Jsonpath-syntax-extensions-v23.patch text/x-patch 48.4 KB

In response to

  • Re: jsonpath at 2019-01-05 00:11:36 from Alexander Korotkov

Responses

  • Re: jsonpath at 2019-01-18 17:58:30 from Alexander Korotkov

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-01-11 22:39:47 Re: problems with foreign keys on partitioned tables
Previous Message Tom Lane 2019-01-11 22:20:33 Re: Three animals fail test-decoding-check on REL_10_STABLE