On Mon, Oct 16, 2017 at 4:41 AM, Dmitry Dolgov <9erthalion6@gmail.com> wrote:
>> On 14 October 2017 at 12:46, Michael Paquier <michael.paquier@gmail.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?
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
