Hello,
On Sat, Oct 19, 2019, 3:27 PM Tomas Vondra <tomas.vondra@xxxxxxxxxxxxxxx> wrote:
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@xxxxxxxxxxxxxxx <mailto:tomas.vondra@xxxxxxxxxxxxxxx>>
>> 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?
Raising an exception at least would prevent people from blanking their column out unintentionally.
And I am willing to write a patch to do that if we have consensus on how to change it.
Ariadne
