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

On 10/12/2017 12:42 AM, Michael Paquier wrote:
> On Wed, Oct 11, 2017 at 11:00 AM, <marko(at)joh(dot)to> wrote:
>> This query fails with an unreasonable error message:
>>
>> =# SELECT jsonb_build_object(VARIADIC '{a,b}'::text[]);
>> ERROR: invalid number of arguments: object must be matched key value
>> pairs
>>
>> jsonb_object(text[]) can be used instead in this case, so perhaps
>> jsonb_build_object() could simply point to that one if a variadic call like
>> this is made?
> It looks like a good idea to do so for this code. It seems that nobody
> has actually bothered testing those functions in more fancy ways than
> the documentation shows... And I think that this is not the only
> problem.
>
> I looked as well at jsonb_build_array(), which also uses VARIADIC ANY,
> being surprised by that:
> =# SELECT jsonb_build_array(variadic '{a,b}'::text[]);
> jsonb_build_array
> -------------------
> [["a", "b"]]
> (1 row)
> But it seems to me that when a variadic call is used, then ["a", "b"]
> is the correct result, no?
>
> The json_* equivalent functions are reacting similarly than the jsonb_* ones.
>
> It is actually possible to make the difference between a variadic and
> a non-variadic call by looking at funcvariadic in
> fcinfo->flinfo->fn_expr. So my suggestion of a fix would be the
> following:
> - refactor jsonb_object with an _internal routine that gets called as
> well for variadic calls of jsonb_build_object. Something equivalent
> needs to be done for the json functions.
> - for json[b]_build_array, let's check if the input is a variadic
> call, then fill in an intermediate structure with all the array
> values, which is used with the existing processing.
>
> More regression tests are needed as well. Andrew, you worked on most
> of those items, including 7e354ab9, what is your opinion on the
> matter?

If the case of an explicit VARIADIC parameter doesn't work the same way
then certainly it's a bug which needs to be fixed, and for which I
apologise. If you have time to produce a patch I will review it. Your
plan sounds good.

cheers

andrew

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