Re: PATCH: recursive json_populate_record()

Nikita Glukhov <n.gluhov@postgrespro.ru>

From: Nikita Glukhov <n.gluhov@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Aleksander Alekseev <a.alekseev@postgrespro.ru>, Michael Paquier <michael.paquier@gmail.com>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2017-01-11T16:54:10Z
Lists: pgsql-hackers

Attachments

On 01/08/2017 09:52 PM, Tom Lane wrote:

> The example you quoted at the start of the thread doesn't fail for me
> in HEAD, so I surmise that it's falling foul of some assertion you added
> in the 0001 patch, but if so I think that assertion is wrong.  attndims
> is really syntactic sugar only and doesn't affect anything meaningful
> semantically.  Here is an example showing why it shouldn't:
>
> regression=# create table foo (d0 _int4, d1 int[], d2 int[3][4]);
> CREATE TABLE
> regression=# select attname,atttypid,attndims from pg_attribute where attrelid = 'foo'::regclass and attnum > 0;
>   attname | atttypid | attndims
> ---------+----------+----------
>   d0      |     1007 |        0
>   d1      |     1007 |        1
>   d2      |     1007 |        2
> (3 rows)
>
> Columns d0,d1,d2 are really all of the same type, and any code that
> treats d0 and d1 differently is certainly broken.
>

Thank you for this example with raw _int4 type.  I didn't expect that
attndims can legally be zero.  There was really wrong assertion "ndims > 0"
that was necessary because I used attndims for verification of a
number of dimensions of a populated json array.

I have fixed the first patch: when the number of dimensionsis unknown
we determine it simply by the number of successive opening brackets at
the start of a json array.  But I'm still using for verification non-zero
ndims values that can be get from composite type attribute (attndims) or
from domain array type (typndims) through specially added function 
get_type_ndims().

On 01/08/2017 09:52 PM, Tom Lane wrote:

> I do not see the point of the second one of these, and it adds no test
> case showing why it would be needed.
I also have added special test cases for json_to_record() showing difference
in behavior that the second patch brings in (see changes in json.out and 
jsonb.out).

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

Commits

  1. Make edge-case behavior of jsonb_populate_record match json_populate_record

  2. Fix thinko in JsObjectSize() macro.

  3. Introduce convenience macros to hide JsonbContainer header accesses better.