Re: jsonb_set() strictness considered harmful to data

On Sun, Oct 20, 2019 at 06:51:05PM -0400, Andrew Dunstan wrote:
>
>On 10/20/19 4:18 PM, Tomas Vondra wrote:
>> On Sun, Oct 20, 2019 at 03:48:05PM -0400, Andrew Dunstan wrote:
>>>
>>> On 10/20/19 1:14 PM, David G. Johnston wrote:
>>>> On Sun, Oct 20, 2019 at 5:31 AM Andrew Dunstan
>>>> <andrew.dunstan@2ndquadrant.com
>>>> <mailto:andrew.dunstan@2ndquadrant.com>> wrote:
>>>>
>>>>     And yet another is to
>>>>     raise an exception, which is easy to write but really punts the
>>>> issue
>>>>     back to the application programmer who will have to decide how to
>>>>     ensure
>>>>     they never pass in a NULL parameter.
>>>>
>>>>
>>>> That's kinda the point - if they never pass NULL they won't encounter
>>>> any problems but as soon as the data and their application don't see
>>>> eye-to-eye the application developer has to decide what they want to
>>>> do about it.  We are in no position to decide for them and making it
>>>> obvious they have a decision to make and implement here doesn't seem
>>>> like a improper position to take.
>>>
>>>
>>> The app dev can avoid this problem today by making sure they don't pass
>>> a NULL as the value. Or they can use a wrapper function which does that
>>> for them. So frankly this doesn't seem like much of an advance. And, as
>>> has been noted, it's not consistent with what either MySQL or MSSQL do.
>>> In general I'm not that keen on raising an exception for cases like
>>> this.
>>>
>>
>> I think the general premise of this thread is that the application
>> developer does not realize that may be necessary, because it's a bit
>> surprising behavior, particularly when having more experience with other
>> databases that behave differently. It's also pretty easy to not notice
>> this issue for a long time, resulting in significant data loss.
>>
>> Let's say you're used to the MSSQL or MySQL behavior, you migrate your
>> application to PostgreSQL or whatever - how do you find out about this
>> behavior? Users are likely to visit
>>
>>    https://www.postgresql.org/docs/12/functions-json.html
>>
>> but that says nothing about how jsonb_set works with NULL values :-(
>
>
>
>We should certainly fix that. I accept some responsibility for the omission.
>

+1

>
>>
>> You're right raising an exception may not be the "right behavior" for
>> whatever definition of "right". But I kinda agree with David that it's
>> somewhat reasonable when we don't know what the "universally correct"
>> thing is (or when there's no such thing). IMHO that's better than
>> silently discarding some of the data.
>
>
>I'm not arguing against the idea of improving the situation. But I am
>arguing against a minimal fix that will not provide much of value to a
>careful app developer. i.e. I want to do more to support app devs.
>Ideally they would not need to use wrapper functions. There will be
>plenty of situations where it is mighty inconvenient to catch an
>exception thrown by jsonb_set(). And catching exceptions can be
>expensive. You want to avoid that if possible in your
>performance-critical plpgsql code.
>

True. And AFAIK catching exceptions is not really possible in some code,
e.g. in stored procedures (because we can't do subtransactions, so no
exception blocks).

>
>>
>> FWIW I think the JSON/JSONB part of our code base is amazing, and the
>> fact that various other databases adopted something very similar over
>> the last couple of years just confirms that. And if this is the only
>> speck of dust in the API, I think that's pretty amazing.
>
>
>TY. When I first saw the SQL/JSON spec I thought I should send a request
>to the SQL standards committee for a royalty payment, since it looked so
>familiar ;-)
>

;-)

>
>>
>> I'm not sure how significant this issue actually is - it's true we got a
>> couple of complaints over the years (judging by a quick search for
>> jsonb_set and NULL in the archives), but I'm not sure that's enough to
>> justify any changes in backbranches. I'd say no, but I have no idea how
>> many people are affected by this but don't know about it ...
>>
>>
>
>No, no backpatching. As I said upthread, this isn't a bug, but it is
>arguably a POLA violation, which is why we should do something for
>release 13.
>

WFM


regards

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