| From: | Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> |
|---|---|
| To: | Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
| Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: PATCH: recursive json_populate_record() |
| Date: | 2016-12-28 00:13:56 |
| Message-ID: | [email protected] |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> 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
| Attachment | Content-Type | Size |
|---|---|---|
| 0001_recursive_json_populate_record_v02.patch | text/x-patch | 94.1 KB |
| 0002_assign_ndims_to_record_function_result_types_v02.patch | text/x-patch | 7.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Thomas Munro | 2016-12-28 00:14:40 | Some thoughts about multi-server sync rep configurations |
| Previous Message | Pavel Stehule | 2016-12-27 23:31:54 | Re: merging some features from plpgsql2 project |