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

On Thu, Oct 12, 2017 at 9:38 PM, Andrew Dunstan
<andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
> 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.

Okay, thanks for your input.

Looking at fmgr.c, I found about get_fn_expr_variadic which is already
doing all the legwork to detect if a call is variadic or not. Also,
after looking at the code I think that using directly
jsonb_object(text[]) is incorrect. If the caller provides for example
int[] as input data, I think that we ought to allow the result to be
casted as a JSON integer. So I have implemented a patch that fills in
intermediate state data when dong a variadic call and feeds that to
the JSONB constructor.

An interesting side-effect of this patch is that:
=# SELECT jsonb_build_object(VARIADIC '{{1,4},{2,5},{3,6}}'::int[][]);
jsonb_build_object
--------------------------
{"1": 4, "2": 5, "3": 6}
(1 row)
This makes actually things more consistent with json_object(), which
generates the same result:
=# SELECT jsonb_object('{{1,4},{2,5},{3,6}}'::text[]);
jsonb_object
--------------------------------
{"1": "4", "2": "5", "3": "6"}
(1 row)
But jsonb_build_object complains for non-variadic calls:
=# SELECT jsonb_build_object('{1,2}'::int[],'{3,4}'::int[]);
ERROR: 22023: key value must be scalar, not array, composite, or json
LOCATION: datum_to_jsonb, jsonb.c:725
So I tend to think that the patch attached is correct, and that we
ought to not complain back to the user if NDIMS > 1 when doing
variadic calls.

More regression tests are added for json[b]_build_object and
json[b]_build_array to validate all that. When deconstructing the
variadic array, there are similarities between the json and jsonb code
but data type evaluate particularly for UNKNOWNOID is different
between both, so things are better not refactoring into a common
routine IMO. Each _array and _object code path also has slight
differences, and it felt more natural to me to not refactor json.c and
jsonb.c to hold a common routine.

In short, I have nailed things down with the attached patch. What do
you guys think?
--
Michael