Re: jsonb_set() strictness considered harmful to data

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Floris Van Nee <florisvannee(at)optiver(dot)com>, Ariadne Conill <ariadne(at)dereferenced(dot)org>, Mark Felder <feld(at)freebsd(dot)org>, "pgsql-generallists(dot)postgresql(dot)org" <pgsql-general(at)lists(dot)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: jsonb_set() strictness considered harmful to data
Date: 2019-10-21 06:07:01
Message-ID: 20191021060701.or6a6a7lg3asixom@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers

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(dot)dunstan(at)2ndquadrant(dot)com
>>>> <mailto:andrew(dot)dunstan(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message Luca Ferrari 2019-10-21 08:52:18 Re: Postgres Point in time Recovery (PITR),
Previous Message Tomas Vondra 2019-10-21 06:01:37 Re: CPU SPIKE

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2019-10-21 06:13:56 CountDBSubscriptions check in dropdb
Previous Message Dilip Kumar 2019-10-21 06:00:14 Re: Questions/Observations related to Gist vacuum