Re: jsonb_set() strictness considered harmful to data

Hello,

On Sat, Oct 19, 2019, 3:27 PM Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
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(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?
>

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