Re: PATCH: recursive json_populate_record()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
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-08 18:52:03
Message-ID: 11489.1483901523@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> writes:
> [ 0001_recursive_json_populate_record_v02.patch ]
> [ 0002_assign_ndims_to_record_function_result_types_v02.patch ]

I do not see the point of the second one of these, and it adds no test
case showing why it would be needed. 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.

So there should be no need to track a particular attndims for an output
column of a function result, and in most cases it's not clear to me where
you would get an attndims value from anyway.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Steve Singer 2017-01-08 19:39:05 Re: sequence data type
Previous Message Tom Lane 2017-01-08 18:36:34 Re: Explicit subtransactions for PL/Tcl