Re: [PATCH] Generic type subscripting

Artur Zakirov <a.zakirov@postgrespro.ru>

From: Arthur Zakirov <a.zakirov@postgrespro.ru>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, David Steele <david@pgmasters.net>, peter.eisentraut@2ndquadrant.com, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2017-03-29T17:14:27Z
Lists: pgsql-hackers
On 28.03.2017 19:31, Dmitry Dolgov wrote:
> On 28 March 2017 at 12:08, Dmitry Dolgov <9erthalion6@gmail.com
> <mailto:9erthalion6@gmail.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. 
https://www.postgresql.org/message-id/CAKNkYnz_WWkzzxyFx934N%3DEp47CAFju-Rk-sGeZo0ui8QdrGmw%40mail.gmail.com

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


Commits

  1. Throw error when assigning jsonb scalar instead of a composite object

  2. Filling array gaps during jsonb subscripting

  3. Implementation of subscripting for jsonb

  4. Allow ALTER TYPE to update an existing type's typsubscript value.

  5. Allow subscripting of hstore values.

  6. Support subscripting of arbitrary types, not only arrays.

  7. jit: Reference function pointer types via llvmjit_types.c.

  8. Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.

  9. jit: Correct parameter type for generated expression evaluation functions.

  10. Renaming for new subscripting mechanism

  11. Fix assertion failure for SSL connections.

  12. Teach eval_const_expressions() to handle some more cases.