Re: jsonb_set() strictness considered harmful to data

On Sat, Oct 19, 2019 at 12:47:39PM -0400, Andrew Dunstan wrote:
>
>On 10/19/19 12:32 PM, David G. Johnston wrote:
>> On Sat, Oct 19, 2019 at 9:19 AM Tomas Vondra
>> <tomas.vondra@2ndquadrant.com <mailto:tomas.vondra@2ndquadrant.com>>
>> wrote:
>>
>>     >
>>     >We invented jsonb_set() (credit to Dmitry Dolgov). And we've had it
>>     >since 9.5. That's five releases ago.  So it's a bit late to be
>>     coming to
>>     >us telling us it's not safe (according to your preconceptions of
>>     what it
>>     >should be doing).
>>     >
>>
>>
>> There have been numerous complaints and questions about this behavior
>> in those five years; and none of the responses to those defenses has
>> actually made the current behavior sound beneficial but rather have
>> simply said "this is how it works, deal with it".
>
>
>I haven't seen a patch, which for most possible solutions should be
>fairly simple to code. This is open source. Code speaks louder than
>complaints.
>

IMHO that might be a bit too harsh - I'm not surprised no one sent a
patch when we're repeatedly telling people "you're holding it wrong".
Without a clear consensus what the "correct" behavior is, I wouldn't
send a patch either.

>
>>
>>     >
>>     >We could change it prospectively (i.e. from release 13 on) if we
>>     choose.
>>     >But absent an actual bug (i.e. acting contrary to documented
>>     behaviour)
>>     >we do not normally backpatch such changes, especially when there is a
>>     >simple workaround for the perceived problem. And it's that policy
>>     that
>>     >is in large measure responsible for Postgres' deserved reputation for
>>     >stability.
>>     >
>>
>>     Yeah.
>>
>>
>> Agreed, this is v13 material if enough people come on board to support
>> making a change.
>
>
>
>We have changed such things in the past. But maybe a new function might
>be a better way to go. I haven't given it enough thought yet.
>

I think the #1 thing we should certainly do is explaining the behavior
in the docs.

>
>
>>
>>     >And if we were to change it I'm not at all sure that we should do
>>     it the
>>     >way that's suggested here, which strikes me as no more intuitive than
>>     >the current behaviour. Rather I think we should possibly fill in
>>     a json
>>     >null in the indicated place.
>>     >
>>
>>     Not sure, but that seems rather confusing to me, because it's
>>     mixing SQL
>>     NULL and JSON null, i.e. it's not clear to me why
>>
>> [...]
>>
>>     But I admit it's quite subjective.
>>
>>
>> Providing SQL NULL to this function and asking it to do something with
>> that is indeed subjective - with no obvious reasonable default, and I
>> agree that "return a NULL" while possible consistent is probably the
>> least useful behavior that could have been chosen.  We should never
>> have allowed an SQL NULL to be an acceptable argument in the first
>> place, and can reasonably safely and effectively prevent it going
>> forward.  Then people will have to explicitly code what they want to
>> do if their data and queries present this invalid unknown data to the
>> function.
>>
>>
>
>How exactly do we prevent a NULL being passed as an argument? The only
>thing we could do would be to raise an exception, I think. That seems
>like a fairly ugly thing to do, I'd need a h3eck of a lot of convincing.
>

I don't know, but if we don't know what the "right" behavior with NULL
is, is raising an exception really that ugly?