Re: BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much

On 10/15/2017 08:08 PM, Michael Paquier wrote:
> On Mon, Oct 16, 2017 at 4:41 AM, Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
>>> On 14 October 2017 at 12:46, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
>>> wrote:
>>>
>>> Okay. Attached is an updated patch fixing what you have reported.
>> Thanks for the patch. Everything looks fine, I just have one question -
>> maybe
>> it makes sense to put this pattern `if (variadic) {/*...*/}` into a separate
>> function? Because from what I see it's basically one part of code (I didn't
>> spot any difference) repeated four times for every function.
> The json/jsonb calls have one difference though. For jsonb, arguments
> with unknown type are enforced to text, which is not the case of json,
> and we don't want to change that behavior. Both the json and jsonb
> calls also use different utility routines which are proper to json.c
> and jsonb.c, so moving all things to jsonfuncs.c is out of the game
> (see add_jsonb and add_json). There is a common place for JSON-common
> code in jsonapi.c, but jsonapi.h is wanted as light-weight (see its
> set of headers), so moving things there is incorrect as well IMO.
>
> One thing that could be done though is to have two routines which
> prepare the arguments for the array and object build, one for each
> file json.c and jsonb.c. The routine could return the number of
> arguments processed, at the exception that if the routine returns -1,
> then the result for the object generated should be NULL, which
> corresponds to the case where a NULL array is given in a variadic
> call.
>
> What do you think?

That seems a reasonable approach.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services