On 3/16/17 11:54 AM, David Steele wrote:
On 2/1/17 12:53 AM, Michael Paquier wrote:
On Thu, Jan 26, 2017 at 6:49 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
Nikita Glukhov <n.glu...@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.

Moved to CF 2017-03 as discussion is going on and more review is
needed on the last set of patches.


The current patches do not apply cleanly at cccbdde:

$ git apply ../other/0001-introduce-JsonContainerXxx-macros-v04.patch
error: patch failed: src/backend/utils/adt/jsonb_util.c:328
error: src/backend/utils/adt/jsonb_util.c: patch does not apply
error: patch failed: src/backend/utils/adt/jsonfuncs.c:1266
error: src/backend/utils/adt/jsonfuncs.c: patch does not apply
error: patch failed: src/include/utils/jsonb.h:218
error: src/include/utils/jsonb.h: patch does not apply

In addition, it appears a number of suggestions have been made by Tom
that warrant new patches.  Marked "Waiting on Author".

This thread has been idle for months since Tom's review.

The submission has been marked "Returned with Feedback". Please feel free to resubmit to a future commitfest.

Thanks,
--
-David
da...@pgmasters.net


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to