Thread: pgsql: Add a non-strict version of jsonb_set

pgsql: Add a non-strict version of jsonb_set

From
Andrew Dunstan
Date:
Add a non-strict version of jsonb_set

jsonb_set_lax() is the same as jsonb_set, except that it takes and extra
argument that specifies what to do if the value argument is NULL. The
default is 'use_json_null'. Other possibilities are 'raise_exception',
'return_target' and 'delete_key', all these behaviours having been
suggested as reasonable by various users.

Discussion: https://postgr.es/m/375873e2-c957-3a8d-64f9-26c43c2b16e7@2ndQuadrant.com

Reviewed by: Pavel Stehule

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/a83586b5543b948f9e621462537a7303b113c482

Modified Files
--------------
doc/src/sgml/func.sgml               | 23 +++++++++++++
src/backend/catalog/system_views.sql |  9 +++++
src/backend/utils/adt/jsonfuncs.c    | 64 ++++++++++++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat      |  3 ++
src/test/regress/expected/jsonb.out  | 57 ++++++++++++++++++++++++++++++++
src/test/regress/sql/jsonb.sql       | 20 +++++++++++
6 files changed, 176 insertions(+)


Re: pgsql: Add a non-strict version of jsonb_set

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> Add a non-strict version of jsonb_set

Shoulda been a catversion bump in here, if only for protocol's sake.

(A useful rule of thumb is "if you won't pass the regression tests
without doing an initdb, there should be a catversion change".)

            regards, tom lane



Re: pgsql: Add a non-strict version of jsonb_set

From
Andrew Dunstan
Date:



> On Jan 17, 2020, at 12:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Andrew Dunstan <andrew@dunslane.net> writes:
>> Add a non-strict version of jsonb_set
>
> Shoulda been a catversion bump in here, if only for protocol's sake.
>
> (A useful rule of thumb is "if you won't pass the regression tests
> without doing an initdb, there should be a catversion change".)
>
>

Argh! Will fix when back at my desk

Cheers

Andrew


Re: pgsql: Add a non-strict version of jsonb_set

From
Andrew Dunstan
Date:
On Fri, Jan 17, 2020 at 1:50 PM Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
>
>
>
> > On Jan 17, 2020, at 12:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Andrew Dunstan <andrew@dunslane.net> writes:
> >> Add a non-strict version of jsonb_set
> >
> > Shoulda been a catversion bump in here, if only for protocol's sake.
> >
> > (A useful rule of thumb is "if you won't pass the regression tests
> > without doing an initdb, there should be a catversion change".)
> >
> >
>
> Argh! Will fix when back at my desk
>


I'd love to have a git pre-commit hook that would warn about this, it
seems to happen several times a year, and I know I've transgressed
more than once. Not sure what the rules should be, something like if
you changed src/include/catalog/* but not
src/include/catalog/catversion.h ?

cheers

andrew

--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: pgsql: Add a non-strict version of jsonb_set

From
Tom Lane
Date:
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> On Jan 17, 2020, at 12:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Shoulda been a catversion bump in here, if only for protocol's sake.

> I'd love to have a git pre-commit hook that would warn about this, it
> seems to happen several times a year, and I know I've transgressed
> more than once. Not sure what the rules should be, something like if
> you changed src/include/catalog/* but not
> src/include/catalog/catversion.h ?

Meh.  I think that would lead to forced catversion bumps even when
not necessary (ex: when just correcting description strings).
The cure could easily be worse than the disease.

In reality, the only reason for repeated catversion bumps during
development is to warn fellow developers that they have to do
an initdb after a git pull.  That's certainly a valuable courtesy,
but the sky generally isn't going to fall if you forget.

I'd be okay with a hook that there was a way to override ("yes,
I know what I'm doing, this doesn't require a catversion change").
But there's no way to do that is there?

            regards, tom lane



Re: pgsql: Add a non-strict version of jsonb_set

From
David Fetter
Date:
On Fri, Jan 17, 2020 at 06:41:02PM -0500, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> >> On Jan 17, 2020, at 12:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>> Shoulda been a catversion bump in here, if only for protocol's sake.
> 
> > I'd love to have a git pre-commit hook that would warn about this, it
> > seems to happen several times a year, and I know I've transgressed
> > more than once. Not sure what the rules should be, something like if
> > you changed src/include/catalog/* but not
> > src/include/catalog/catversion.h ?
> 
> Meh.  I think that would lead to forced catversion bumps even when
> not necessary (ex: when just correcting description strings).
> The cure could easily be worse than the disease.
> 
> In reality, the only reason for repeated catversion bumps during
> development is to warn fellow developers that they have to do
> an initdb after a git pull.  That's certainly a valuable courtesy,
> but the sky generally isn't going to fall if you forget.
> 
> I'd be okay with a hook that there was a way to override ("yes,
> I know what I'm doing, this doesn't require a catversion change").
> But there's no way to do that is there?

I'm pretty sure there is. The program called by the commit hook can
prompt the user for input. Something that says 

    Are you sure you want to push this without a catalog bump? (N/y)

with the corresponding machinery for defaults would do it.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate



Re: pgsql: Add a non-strict version of jsonb_set

From
Tom Lane
Date:
David Fetter <david@fetter.org> writes:
> On Fri, Jan 17, 2020 at 06:41:02PM -0500, Tom Lane wrote:
>> I'd be okay with a hook that there was a way to override ("yes,
>> I know what I'm doing, this doesn't require a catversion change").
>> But there's no way to do that is there?

> I'm pretty sure there is. The program called by the commit hook can
> prompt the user for input.

... um, but the commit hook in question would run on the repo server.
Is there really something in the git protocol that would allow sending
this sort of interaction back to the person running "git push"?

I think you are right that individual committers could set up such hooks
in their own private repos.  But that's not what was being suggested,
or so I thought.

            regards, tom lane



Re: pgsql: Add a non-strict version of jsonb_set

From
David Fetter
Date:
On Sun, Jan 19, 2020 at 12:46:19PM -0500, Tom Lane wrote:
> David Fetter <david@fetter.org> writes:
> > On Fri, Jan 17, 2020 at 06:41:02PM -0500, Tom Lane wrote:
> >> I'd be okay with a hook that there was a way to override ("yes,
> >> I know what I'm doing, this doesn't require a catversion change").
> >> But there's no way to do that is there?
> 
> > I'm pretty sure there is. The program called by the commit hook can
> > prompt the user for input.
> 
> ... um, but the commit hook in question would run on the repo server.
> Is there really something in the git protocol that would allow sending
> this sort of interaction back to the person running "git push"?

According to the folks I consulted, that would require special magic
on the client side.

How about something that sent an email or otherwise signaled the
person pushing that this situation obtained? It's not ideal, but it's
centralized, so harder to get inconsistent.

> I think you are right that individual committers could set up such hooks
> in their own private repos.  But that's not what was being suggested,
> or so I thought.

Distributing the problem by asking people to have those hooks on their
private repo seems like a suboptimal move. Change dev boxes, and watch
your carefully structured safeguards vanish.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate



Re: pgsql: Add a non-strict version of jsonb_set

From
Mark Dilger
Date:

> On Jan 19, 2020, at 12:20 PM, David Fetter <david@fetter.org> wrote:
>
> On Sun, Jan 19, 2020 at 12:46:19PM -0500, Tom Lane wrote:
>> David Fetter <david@fetter.org> writes:
>>> On Fri, Jan 17, 2020 at 06:41:02PM -0500, Tom Lane wrote:
>>>> I'd be okay with a hook that there was a way to override ("yes,
>>>> I know what I'm doing, this doesn't require a catversion change").
>>>> But there's no way to do that is there?
>>
>>> I'm pretty sure there is. The program called by the commit hook can
>>> prompt the user for input.
>>
>> ... um, but the commit hook in question would run on the repo server.
>> Is there really something in the git protocol that would allow sending
>> this sort of interaction back to the person running "git push"?
>
> According to the folks I consulted, that would require special magic
> on the client side.
>
> How about something that sent an email or otherwise signaled the
> person pushing that this situation obtained? It's not ideal, but it's
> centralized, so harder to get inconsistent.
>
>> I think you are right that individual committers could set up such hooks
>> in their own private repos.  But that's not what was being suggested,
>> or so I thought.
>
> Distributing the problem by asking people to have those hooks on their
> private repo seems like a suboptimal move. Change dev boxes, and watch
> your carefully structured safeguards vanish.

I disagree.  If you have a committed directory containing hook scripts, and
a script in src/tools that can instantiate the hooks of your choosing in your
development repository, then client-side hooks can be shared between
developers without forcing any developer to use any particular hook.  I
have been annoyed in the past that .git/hooks is not itself under version
control, so shared hook scripts need to go elsewhere for versioning, and be
optionally copied to .git/hooks by some new tool, say src/tools/make_hooks.

Client side hooks of this kind could be useful for developers without
community commit privileges, since local repository commits could also be
rejected, perhaps with an “are you sure” type override option, and more
than a catversion checking hook would be possible.

I have wanted a hook to check code indentation [1], and not all developers
agreed it would be useful enough to be worth the pain of rejecting commits.
If it were a client side hook, and had “are you sure” logic rather than
rejecting the commit unconditionally, it might be more popular.  Either way,
each developer could opt-in or opt-out as they like when running make_hooks.

Does anybody object to this idea in principle?  If not, I can put something
like this together from a similar script I already have lying around.


[1] https://www.postgresql.org/message-id/7825.1511902037%40sss.pgh.pa.us

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






Re: pgsql: Add a non-strict version of jsonb_set

From
Tom Lane
Date:
Mark Dilger <mark.dilger@enterprisedb.com> writes:
> On Jan 19, 2020, at 12:20 PM, David Fetter <david@fetter.org> wrote:
>> On Sun, Jan 19, 2020 at 12:46:19PM -0500, Tom Lane wrote:
>>> I think you are right that individual committers could set up such hooks
>>> in their own private repos.  But that's not what was being suggested,
>>> or so I thought.

> Client side hooks of this kind could be useful for developers without
> community commit privileges, since local repository commits could also be
> rejected, perhaps with an “are you sure” type override option, and more
> than a catversion checking hook would be possible.

Yeah, the point that this could be useful to non-committer contributors
seems pretty compelling.  The catversion complaint in particular would
*not* be useful, since you're generally not supposed to include catversion
bumps in submitted patches --- but I can easily envision that other
check-hooks might be of widespread use.

            regards, tom lane



Re: pgsql: Add a non-strict version of jsonb_set

From
Andrew Dunstan
Date:
On Mon, Jan 20, 2020 at 4:16 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> David Fetter <david@fetter.org> writes:
> > On Fri, Jan 17, 2020 at 06:41:02PM -0500, Tom Lane wrote:
> >> I'd be okay with a hook that there was a way to override ("yes,
> >> I know what I'm doing, this doesn't require a catversion change").
> >> But there's no way to do that is there?
>
> > I'm pretty sure there is. The program called by the commit hook can
> > prompt the user for input.
>
> ... um, but the commit hook in question would run on the repo server.
> Is there really something in the git protocol that would allow sending
> this sort of interaction back to the person running "git push"?
>
> I think you are right that individual committers could set up such hooks
> in their own private repos.  But that's not what was being suggested,
> or so I thought.
>

I was really thinking of a client side hook. The reason I started the
discussion is so I could institutionalize the knowledge, e.g. by
adding something to the wiki.

I was also thinking of something that would give a warning rather than
reject the commit. Then the committer could ignore it or not as they
choose.  Something like that might also be valuable on the server
side, since it would mean all committers would get the warning when
appropriate, but it might not be worth the trouble.

I agree that we don't want something server side that would reject a
commit on this basis.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: pgsql: Add a non-strict version of jsonb_set

From
Tom Lane
Date:
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On Mon, Jan 20, 2020 at 4:16 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think you are right that individual committers could set up such hooks
>> in their own private repos.  But that's not what was being suggested,
>> or so I thought.

> I was really thinking of a client side hook. The reason I started the
> discussion is so I could institutionalize the knowledge, e.g. by
> adding something to the wiki.

Ah, that makes sense.  Although Mark's idea of including a library
of possible hooks somewhere under src/tools/ seems even better.

> I was also thinking of something that would give a warning rather than
> reject the commit. Then the committer could ignore it or not as they
> choose.  Something like that might also be valuable on the server
> side, since it would mean all committers would get the warning when
> appropriate, but it might not be worth the trouble.

Another point is that a server-side hook is really too late: you'd
rather get the notice before you "git push", not after.

            regards, tom lane



Re: pgsql: Add a non-strict version of jsonb_set

From
Robert Haas
Date:
On Sun, Jan 19, 2020 at 7:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ah, that makes sense.  Although Mark's idea of including a library
> of possible hooks somewhere under src/tools/ seems even better.

+1. And directions for setting them up (or, as I think he was
proposing, a script, but directions might be better).

> Another point is that a server-side hook is really too late: you'd
> rather get the notice before you "git push", not after.

Yeah, getting warned at commit time would be good.

Another thing that would be nice is to warn about commits that don't
include a "Description:" line. I have been trying to be religious
about that, but I seem to periodically apostatize.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: pgsql: Add a non-strict version of jsonb_set

From
Robert Haas
Date:
On Tue, Jan 28, 2020 at 10:36 AM Robert Haas <robertmhaas@gmail.com> wrote:
> Another thing that would be nice is to warn about commits that don't
> include a "Description:" line. I have been trying to be religious
> about that, but I seem to periodically apostatize.

And of course I meant "Discussion". :-(

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: pgsql: Add a non-strict version of jsonb_set

From
Michael Paquier
Date:
On Tue, Jan 28, 2020 at 10:37:00AM -0500, Robert Haas wrote:
> On Tue, Jan 28, 2020 at 10:36 AM Robert Haas <robertmhaas@gmail.com> wrote:
>> Another thing that would be nice is to warn about commits that don't
>> include a "Description:" line. I have been trying to be religious
>> about that, but I seem to periodically apostatize.
>
> And of course I meant "Discussion". :-(

That would be very nice.
--
Michael

Attachment

Re: pgsql: Add a non-strict version of jsonb_set

From
Magnus Hagander
Date:
On Tue, Jan 28, 2020 at 4:36 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Sun, Jan 19, 2020 at 7:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Ah, that makes sense.  Although Mark's idea of including a library
> > of possible hooks somewhere under src/tools/ seems even better.
>
> +1. And directions for setting them up (or, as I think he was
> proposing, a script, but directions might be better).
>
> > Another point is that a server-side hook is really too late: you'd
> > rather get the notice before you "git push", not after.
>
> Yeah, getting warned at commit time would be good.

For warnings, the server-side hook is definitely too late -- as those
are things end up being accepted anyway, and since it's git you cannot
amend them after you've pushed (cleanly).

Server side hooks are appropriate for things that should be outright
refused (like we for example refuse pushes from committers who have
misconfigured their git client -- it has happened a few times that
we've rejected pushes with commits from user@localhost or similar
instead of their registered email address). In those cases the commit
is completely rejected with an error, not a warning, so there's a big
difference.


> Another thing that would be nice is to warn about commits that don't
> include a "Description:" line. I have been trying to be religious
> about that, but I seem to periodically apostatize.

We'd also have to come to this magic actual agreement on which such
fields we *want* :)  Which alone would be an improvement.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/



Re: pgsql: Add a non-strict version of jsonb_set

From
Michael Paquier
Date:
On Wed, Jan 29, 2020 at 09:54:23AM +0100, Magnus Hagander wrote:
> On Tue, Jan 28, 2020 at 4:36 PM Robert Haas <robertmhaas@gmail.com> wrote:
>> Another thing that would be nice is to warn about commits that don't
>> include a "Description:" line. I have been trying to be religious
>> about that, but I seem to periodically apostatize.

I had to look at a dictionary for this one..

> We'd also have to come to this magic actual agreement on which such
> fields we *want* :)  Which alone would be an improvement.

Well, there has been some discussion about this matter on the lists
not so long ago:
https://www.postgresql.org/message-id/CABUevEzauoMm3irCyg4s=vUX5hUcKmTW_R69fiKS4+72gzyAOw@mail.gmail.com
https://www.postgresql.org/message-id/CA+hUKGJR+YtZUdq9_6cFsqCRH_fvfumdDZgmiLwXBHCrTJUGmw@mail.gmail.com

And it seems to me that the only consensus we reached was about having
"Discussion:", while the other tags were not that popular.
--
Michael

Attachment

Re: pgsql: Add a non-strict version of jsonb_set

From
Tom Lane
Date:
Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Jan 29, 2020 at 09:54:23AM +0100, Magnus Hagander wrote:
>> We'd also have to come to this magic actual agreement on which such
>> fields we *want* :)  Which alone would be an improvement.

> Well, there has been some discussion about this matter on the lists
> not so long ago:
> https://www.postgresql.org/message-id/CABUevEzauoMm3irCyg4s=vUX5hUcKmTW_R69fiKS4+72gzyAOw@mail.gmail.com
> https://www.postgresql.org/message-id/CA+hUKGJR+YtZUdq9_6cFsqCRH_fvfumdDZgmiLwXBHCrTJUGmw@mail.gmail.com
> And it seems to me that the only consensus we reached was about having
> "Discussion:", while the other tags were not that popular.

Yeah, that's my understanding of the state of affairs too:
a Discussion: link is expected if relevant, but whether you want to
express other info as a tag or free text is committer's choice.

            regards, tom lane