Re: PATCH: recursive json_populate_record()

Nikita Glukhov <n.gluhov@postgrespro.ru>

From: Nikita Glukhov <n.gluhov@postgrespro.ru>
To: Aleksander Alekseev <a.alekseev@postgrespro.ru>, Michael Paquier <michael.paquier@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2016-12-28T00:13:56Z
Lists: pgsql-hackers

Attachments

 > I've noticed that this patch is on CF and needs a reviewer so I decided
 > to take a look. Code looks good to me in general, it's well covered by
 > tests and passes `make check-world`.

Thanks for your review.

 > However it would be nice to have a little more comments. In my opinion
 > every procedure have to have at least a short description - what it
 > does, what arguments it receives and what it returns, even if it's a
 > static procedure. Same applies for structures and their fields.

I have added some comments for functions and structures in the second 
version of this patch.

 > Another thing that bothers me is a FIXME comment:
 >
 > ```
 > tupdesc = lookup_rowtype_tupdesc(type, typmod); /* FIXME cache */
 > ```
 >
 > I suggest remove it or implement a caching here as planned.

I have implemented tuple descriptor caching here in populate_composite() 
and also in populate_record_worker() (by using populate_composite() 
instead of populate_record()). These improvements can speed up bulk 
jsonb conversion by 15-20% in the simple test with two nested records.

-- 
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.