Re: jsonb_concat: make sure we always return a non-scalar value - Mailing list pgsql-hackers

From Oskari Saarenmaa
Subject Re: jsonb_concat: make sure we always return a non-scalar value
Date
Msg-id 55F0667E.3070804@ohmu.fi
Whole thread Raw
In response to Re: jsonb_concat: make sure we always return a non-scalar value  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: jsonb_concat: make sure we always return a non-scalar value
Re: jsonb_concat: make sure we always return a non-scalar value
List pgsql-hackers
09.09.2015, 18:53, Andrew Dunstan kirjoitti:
> On 09/08/2015 09:54 AM, Andrew Dunstan wrote:
>> On 09/05/2015 02:47 AM, Oskari Saarenmaa wrote:
>>> There was a long thread about concatenating jsonb objects to each
>>> other, but that discussion didn't touch concatenating other types.
>>> Currently jsonb_concat always just returns the other argument if one
>>> of arguments is considered empty.  This causes surprising behavior
>>> when concatenating scalar values to empty arrays:
>>>
>>> os=# select '[]'::jsonb || '1'::jsonb;
>>> 1
>>>
>>> os=# select '[]'::jsonb || '[1]'::jsonb;
>>>  [1]
>>>
>>> os=# select '[]'::jsonb || '1'::jsonb || '2'::jsonb;
>>>  [1, 2]
>>>
>>> os=# select '0'::jsonb || '1'::jsonb;
>>>  [0, 1]
>>>
>>> os=# select '{"x": "y"}'::jsonb || '[1]'::jsonb;
>>>  [{"x": "y"}, 1]
>>>
>>> os=# select '{"x": "y"}'::jsonb || '1'::jsonb;
>>> ERROR:  invalid concatenation of jsonb objects
>>>
>>> Attached a patch to fix and test this.  Also added a test case for
>>> concatenating two scalar values which currently produces an array..
>>> I'm not sure that behavior makes sense, but didn't want to change
>>> that in this patch as I guess someone could consider that feature
>>> useful.
>>
>>
>>
>> This looks correct. Barring objection I'll apply this shortly.
>
>
> Actually, I'm not sure the test is sufficient here. It looks to me like
> we should only be taking this fast path if one operand is an empty array
> and the other is a non-scalar array.
>
> Otherwise we get things like this (second case is wrong, I think):
>
>     andrew=# select '[]'::jsonb || '"a"';
>       ?column?
>     ----------
>       ["a"]
>
>     andrew=# select '[]'::jsonb || '{"a":"b"}';
>        ?column?
>     ------------
>       {"a": "b"}
>
>     andrew=# select '[1]'::jsonb || '{"a":"b"}';
>          ?column?
>     -----------------
>       [1, {"a": "b"}]

It looks wrong, but I'm not sure what's right in that case.  I think 
it'd make sense to just restrict concatenation to object || object, 
array || array and array || scalar and document that.  I doubt many 
people expect their objects to turn into arrays if they accidentally 
concatenate an array into it.  Alternatively the behavior could depend 
on the order of arguments for concatenation, array || anything -> array, 
object || array -> error.

/ Oskari



pgsql-hackers by date:

Previous
From: Robbie Harwood
Date:
Subject: Re: [PATCH v2] GSSAPI encryption support
Next
From: "周正中(德歌)"
Date:
Subject: 答复:[HACKERS] 答复:[HACKERS] 答复:[HACKERS] about fsync in CLOG buffer write