Re: PATCH: recursive json_populate_record()

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 <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> 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.
>>
>> 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
david(at)pgmasters(dot)net