From: | Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PATCH: recursive json_populate_record() |
Date: | 2017-01-11 16:54:10 |
Message-ID: | 687e19a7-14e2-d0f5-2bff-35d62bc5c05a@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0001_recursive_json_populate_record_v03.patch | text/x-patch | 102.9 KB |
0002_assign_ndims_to_record_function_result_types_v03.patch | text/x-patch | 9.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-01-11 16:55:15 | Re: [HACKERS] Questionable tag usage |
Previous Message | Stephen Frost | 2017-01-11 16:48:25 | Re: pg_restore accepts -j -1 |