Thread: jsonb array-style subscription

jsonb array-style subscription

From
Dmitry Dolgov
Date:
Hi,

Here is a reworked version of patch for jsonb subscription.
There weren't many changes in functionality:

=# create TEMP TABLE test_jsonb_subscript (
       id int,
       test_json jsonb
);

=# insert into test_jsonb_subscript values
(1, '{}'),
(2, '{}');

=# update test_jsonb_subscript set test_json['a'] = 42;
=# select * from test_jsonb_subscript;
 id |        test_json         
----+--------------------------
  1 | {"a": 42}
  2 | {"a": 42}
(2 rows)

=# select test_json['a'] from test_jsonb_subscript;
 test_json  
------------
 {"a": 42}
 {"a": 42}
(2 rows)

I've cleaned up the code, created a separate JsonbRef node (and there are a lot of small changes because of that), abandoned an idea of "deep nesting" of assignments (because it doesn't relate to jsonb subscription, is more about the
"jsonb_set" function, and anyway it's not a good idea). It looks fine for me, and I need a little guidance - is it ok to propose this feature for commitfest 2016-03 for a review?
Attachment

Re: jsonb array-style subscription

From
Michael Paquier
Date:
On Mon, Jan 18, 2016 at 9:32 PM, Dmitry Dolgov <9erthalion6@gmail.com> wrote:
> I've cleaned up the code, created a separate JsonbRef node (and there are a
> lot of small changes because of that), abandoned an idea of "deep nesting"
> of assignments (because it doesn't relate to jsonb subscription, is more
> about the
> "jsonb_set" function, and anyway it's not a good idea). It looks fine for
> me, and I need a little guidance - is it ok to propose this feature for
> commitfest 2016-03 for a review?

That's what the CF is for. If you are confident enough, I think that
you should register it, elsewhise the fate of this patch will be
likely to fall in oblivion.
-- 
Michael



Re: jsonb array-style subscription

From
Alvaro Herrera
Date:
Dmitry Dolgov wrote:

> I've cleaned up the code, created a separate JsonbRef node (and there are a
> lot of small changes because of that), abandoned an idea of "deep nesting"
> of assignments (because it doesn't relate to jsonb subscription, is more
> about the
> "jsonb_set" function, and anyway it's not a good idea). It looks fine for
> me, and I need a little guidance - is it ok to propose this feature for
> commitfest 2016-03 for a review?

Has this patch been proposed in some commitfest previously?  One of the
less-commonly-invoked rules of commitfests is that you can't add patches
that are too invasive to the last one -- so your last chance for 9.6 was
2016-01.  This is harsh to patch submitters, but it helps keep the size
of the last commitfest down to a reasonable level; otherwise we are
never able to finish it.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: jsonb array-style subscription

From
Dmitry Dolgov
Date:
On 20 January 2016 at 02:14, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Dmitry Dolgov wrote:

> I've cleaned up the code, created a separate JsonbRef node (and there are a
> lot of small changes because of that), abandoned an idea of "deep nesting"
> of assignments (because it doesn't relate to jsonb subscription, is more
> about the
> "jsonb_set" function, and anyway it's not a good idea). It looks fine for
> me, and I need a little guidance - is it ok to propose this feature for
> commitfest 2016-03 for a review?

Has this patch been proposed in some commitfest previously?
 
Unfortunately, it wasn't. I just sent an intermediate version of this patch to hackers several months ago to discuss it.

you can't add patches that are too invasive to the last one -- so your last chance for 9.6 was 2016-01.

Yes, that's what I was worried about (although I didn't know exactly about this rule before).

Re: jsonb array-style subscription

From
Vitaly Burovoy
Date:
On 1/19/16, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Dmitry Dolgov wrote:
>
>> I've cleaned up the code, created a separate JsonbRef node (and there are
>> a
>> lot of small changes because of that), abandoned an idea of "deep
>> nesting"
>> of assignments (because it doesn't relate to jsonb subscription, is more
>> about the
>> "jsonb_set" function, and anyway it's not a good idea). It looks fine for
>> me, and I need a little guidance - is it ok to propose this feature for
>> commitfest 2016-03 for a review?
>
> Has this patch been proposed in some commitfest previously?  One of the
> less-commonly-invoked rules of commitfests is that you can't add patches
> that are too invasive to the last one -- so your last chance for 9.6 was
> 2016-01.  This is harsh to patch submitters, but it helps keep the size
> of the last commitfest down to a reasonable level; otherwise we are
> never able to finish it.

I'd like to be a reviewer for the patch. It does not look big and very invasive.

Is it a final decision or it has a chance? If something there hurts
committers, it can end up as "Rejected with feedback" (since the patch
is already in the CF[1])?

[1]https://commitfest.postgresql.org/9/485/
-- 
Best regards,
Vitaly Burovoy



Re: jsonb array-style subscription

From
Tom Lane
Date:
Vitaly Burovoy <vitaly.burovoy@gmail.com> writes:
> I'd like to be a reviewer for the patch. It does not look big and very invasive.
> Is it a final decision or it has a chance? If something there hurts
> committers, it can end up as "Rejected with feedback" (since the patch
> is already in the CF[1])?

Well, it is pretty invasive, and I'm not sure anyone has bought in on the
design.  The problem I've got with it is that it's a one-off that embeds
a whole lot of new parser/planner/executor infrastructure to serve exactly
one datatype, ie jsonb.  That does not seem like a good design approach,
nor in keeping with the way Postgres usually goes at things.

If the patch were proposing a similar amount of new infrastructure to
support some datatype-extensible concept of subscripting, I'd be much
happier about it.

I believe there's been some handwaving in the past about extensible
approaches to subscripting, though I haven't got time to troll the
archives for it right now.
        regards, tom lane



Re: jsonb array-style subscription

From
Jim Nasby
Date:
On 3/2/16 6:24 PM, Tom Lane wrote:
> If the patch were proposing a similar amount of new infrastructure to
> support some datatype-extensible concept of subscripting, I'd be much
> happier about it.

+1

> I believe there's been some handwaving in the past about extensible
> approaches to subscripting, though I haven't got time to troll the
> archives for it right now.

I'd be able to make use of that in my ndarray data type. It would also 
be nice to be able to add things like matrix types, sparse arrays, and 
variable size arrays (ie: list of lists), and subscripting is how you'd 
want to interface with all of those.

Presumably the point type is handled specially today, so that should be 
taken care off too.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com



Re: jsonb array-style subscription

From
Dmitry Dolgov
Date:
<div dir="ltr"><div class="gmail_extra"><span style="font-size:12.8px">> If the patch were proposing a similar
amountof new infrastructure to</span><br style="font-size:12.8px" /><span style="font-size:12.8px">> support some
datatype-extensibleconcept of subscripting, I'd be much</span><br style="font-size:12.8px" /><span
style="font-size:12.8px">>happier about it.<br /></span><br />Well, actually, I agree with that. I can try to rework
thepatch to achieve this goal.</div></div> 

Re: jsonb array-style subscription

From
Peter Geoghegan
Date:
On Thu, Mar 3, 2016 at 2:31 AM, Dmitry Dolgov <9erthalion6@gmail.com> wrote:
> Well, actually, I agree with that. I can try to rework the patch to achieve
> this goal.

Good idea.

I wonder, having taken a quick look at the patch, how this works?:

+select * from test_jsonb_subscript where
test_json['key_doesnt_exists'] = '"value"';
+ id | test_json
+----+-----------
+(0 rows)

Can this use an index, in principle? If not, do you have a plan to get
it to a place where it can? How can the expression be made to map on
to existing indexable operators, such as the containment operator @>,
or even B-Tree opclass operators like = or <=, for example?

This kind of thing was always my main problem with jsonb array-style
subscription. I think it's really quite desirable in theory, but I
also think that these problems need to be fixed first. Think that I
made this point before.

ISTM that these expressions need to be indexable in some way, which
seems like a significantly harder project, especially because the
mapping between an expression in a predicate like this and an
indexable operator like @> is completely non-obvious. Making such a
mapping itself extensible seems even more tricky, which is what it
would take, I suspect. Indexing is always of great importance for
jsonb. It's already too complicated.

-- 
Peter Geoghega



Re: jsonb array-style subscription

From
David Steele
Date:
Hi Dmitry,

On 3/3/16 5:31 AM, Dmitry Dolgov wrote:
>> If the patch were proposing a similar amount of new infrastructure to
>> support some datatype-extensible concept of subscripting, I'd be much
>> happier about it.
> 
> Well, actually, I agree with that. I can try to rework the patch to
> achieve this goal.

Do you have an updated patch ready?  If so please post it by Monday or
this patch will be marked "returned with feedback".

Thanks,
-- 
-David
david@pgmasters.net



Re: jsonb array-style subscription

From
Dmitry Dolgov
Date:
<div dir="ltr">> <span style="font-size:12.8px">Do you have an updated patch ready?<br /><br />No, I'm afraid it
willnot be ready for Monday.</span></div> 

Re: jsonb array-style subscription

From
David Steele
Date:
On 3/20/16 2:29 PM, Dmitry Dolgov wrote:
>  > Do you have an updated patch ready?
>
> No, I'm afraid it will not be ready for Monday.

I have marked this "returned with feedback".  Please feel free to submit 
a reworked patch for 9.7!

-- 
-David
david@pgmasters.net