Thread: jsonb array-style subscription

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

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

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

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

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

<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> 

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

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

<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> 

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