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@xxxxxxxxxxxxxxx >>> <mailto:andrew.dunstan@xxxxxxxxxxxxxxx>> 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. > > 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. > > 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. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
