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(dot)vondra(at)2ndquadrant(dot)com <mailto:tomas(dot)vondra(at)2ndquadrant(dot)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?

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services