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-25 21:49:43
Message-ID: 16179.1485380983@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:
> 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2017-01-25 21:51:37 Re: patch: function xmltable
Previous Message Michael Paquier 2017-01-25 21:43:05 Re: multivariate statistics (v19)