Re: PATCH: recursive json_populate_record()

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nikita Glukhov <n.gluhov@postgrespro.ru>
Cc: Aleksander Alekseev <a.alekseev@postgrespro.ru>, Michael Paquier <michael.paquier@gmail.com>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2017-01-25T21:49:43Z
Lists: pgsql-hackers
Nikita Glukhov <n.gluhov@postgrespro.ru> writes:
> On 25.01.2017 23:58, Tom Lane wrote:
>> I think you need to take a second look at the code you're producing
>> and realize that it's not so clean either.  This extract from
>> populate_record_field, for example, is pretty horrid:

> But what if we introduce some helper macros like this:

> #define JsValueIsNull(jsv) \
>      ((jsv)->is_json ? !(jsv)->val.json.str \
>          : !(jsv)->val.jsonb || (jsv)->val.jsonb->type == jbvNull)

> #define JsValueIsString(jsv) \
>      ((jsv)->is_json ? (jsv)->val.json.type == JSON_TOKEN_STRING \
>          : (jsv)->val.jsonb && (jsv)->val.jsonb->type == jbvString)

Yeah, I was wondering about that too.  I'm not sure that you can make
a reasonable set of helper macros that will fix this, but if you want
to try, go for it.

BTW, just as a stylistic thing, I find "a?b:c||d" unreadable: I have
to go back to the manual every darn time to convince myself whether
that means (a?b:c)||d or a?b:(c||d).  It's better not to rely on
the reader (... or the author) having memorized C's precedence rules
in quite that much detail.  Extra parens help.

			regards, tom lane


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.