From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: jsonb_delete with arrays |
Date: | 2017-01-17 11:45:08 |
Message-ID: | CABUevEyFj=X7b3iyW223Ty-HCKMhXjn1hU8JffOwJhrdDoYGVA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jan 17, 2017 at 8:25 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:
> On Sun, Dec 18, 2016 at 1:27 AM, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
> wrote:
> > Speaking about implementation of `jsonb_delete_array` - it's fine, but I
> > would like to suggest two modifications:
> >
> > * create a separate helper function for jsonb delete operation, to use
> it in
> > both `jsonb_delete` and `jsonb_delete_array`. It will help to concentrate
> > related logic in one place.
>
> I am not sure that it is much a win as the code loses readability for
> a minimal refactoring. What would have been nice is to group as well
> jsonb_delete_idx but handling of the element deletion is really
> different there.
>
I agree with that. I agree with investigating it as an option, but I think
the lost readability is worse.
>
> > * use variadic arguments for `jsonb_delete_array`. For rare cases, when
> > someone decides to use this function directly instead of corresponding
> > operator. It will be more consistent with `jsonb_delete` from my point of
> > view, because it's transition from `jsonb_delete(data, 'key')` to
> > `jsonb_delete(data, 'key1', 'key2')` is more smooth, than to
> > `jsonb_delete(data, '{key1, key2}')`.
>
> That's a good idea.
>
I can see the point of that. In the particular usecase I built it for
originally though, the list of keys came from the application, in which
case binding them as an array was a lot more efficient (so as not to
require a whole lot of different prepared statements, one for each number
of parameters). But that should be workaround-able using the VARIADIC
keyword in the caller. Or by just using the operator.
> > I've attached a patch with these modifications. What do you think?
>
> Looking at both patches proposed, documentation is still missing in
> the list of jsonb operators as '-' is missing for arrays. I am marking
> this patch as waiting on author for now.
>
Added in updated patch. Do you see that as enough, or do we need it in some
more places in the docs as well?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Attachment | Content-Type | Size |
---|---|---|
jsonb_delete_array_2.patch | text/x-patch | 7.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Rushabh Lathia | 2017-01-17 11:49:57 | Re: Gather Merge |
Previous Message | Michael Paquier | 2017-01-17 11:35:57 | Re: Support for pg_receivexlog --format=plain|tar |