Re: [PATCH] Generic type subscripting

On 28.03.2017 19:31, Dmitry Dolgov wrote:
> On 28 March 2017 at 12:08, Dmitry Dolgov <9erthalion6(at)gmail(dot)com
> <mailto:9erthalion6(at)gmail(dot)com>> wrote:
>>
>> Wow, I didn't notice that, sorry - will fix it shortly.
>
> So, here is the corrected version of the patch.

I have some picky comments.

I'm not sure that "typsbsparse" is better than "typsubscripting" or
"typsubparse". Maybe "typsubsparse"?

> <row>
> + <entry><structfield>typsubscripting</structfield></entry>
> + <entry><type>regproc</type></entry>

Here you didn't fix "typsubscripting" to new name.

> + <title>JSON subscripting</title>
> + <para>
> + JSONB data type support array-style subscripting expressions to extract or update particular element. An example of subscripting syntax:

Should be "JSONB data type supports".

> + to handle subscripting expressions. It should contains logic for verification
> + and decide which function must be used for evaluation of this expression.
> + For instance:

Should be "It should contain".

> + <sect2 id="json-subscripting">
> + <title>JSON subscripting</title>
> + <para>
> + JSONB data type support array-style subscripting expressions to extract or update particular element. An example of subscripting syntax:

You have implemented jsonb subscripting. The documentation should be
fixed to:

+ <sect2 id="jsonb-subscripting">
+ <title><type>jsonb</> Subscripting</title>
+ <para>
+ <type>jsonb</> data type support array-style subscripting
expressions to extract or update particular element. An example of
subscripting syntax:

I think IsOneOf() macros should be removed. Since it is not used anywhere.

> + Assert(subexpr != NULL);
> +
> + if (subexpr == NULL)
> + ereport(ERROR,
> + (errcode(ERRCODE_DATATYPE_MISMATCH),
> + errmsg("jsonb subscript does not support slices"),
> + parser_errposition(pstate, exprLocation(
> + ((Node *) lfirst(sbsref->refupperindexpr->head))))));

Here one of the conditions is excess. Better to delete assert condition
I think.

I've tried tests from message [1]. They looks good. Performance looks
similar for subscripting without patch and with patch.

I wanted to implement subscripting for ltree or hstore extensions.
Subscripting for ltree looks more interesting. Especially with slicing.
But I haven't done it yet. I hope that I will implement it tomorrow.

1.
/message-id/CAKNkYnz_WWkzzxyFx934N%3DEp47CAFju-Rk-sGeZo0ui8QdrGmw%40mail.gmail.com

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company