Re: [PATCH] Generic type subscripting

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: Oleg Bartunov <obartunov(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Steele <david(at)pgmasters(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Generic type subscripting
Date: 2017-11-11 15:34:31
Message-ID: CA+q6zcX5_meFnpnQj=wY5iyNXs-i_x+KpJ8eyZGnvn3CaUS2YQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

> On 8 November 2017 at 17:25, Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
wrote:

Thanks for your review!

> > On Tue, Nov 07, 2017 at 09:00:43PM +0100, Dmitry Dolgov wrote:
> > > +Datum
> > > +custom_subscripting_parse(PG_FUNCTION_ARGS)
> > > +{
> > > + bool isAssignment =
PG_GETARG_BOOL(0);
> >
> > Here isAssignment is unused variable, so it could be removed.
>
> In this case I disagree - the purpose of these examples is to show
everything
> you can use. So I just need to come up with some example that involves
> `isAssignment`.

I've incorporated this variable into the tutorial.

> To be more specific I attached the patch 0005-Fix-ExprEvalStep.patch,
which can be applyed over your patches.

Oh, now I see, thank you.

> I think you forgot commas and conjunction 'and'.
> Here you forgot comma or 'and'. Also 'contain' should be used instead
'contains'.
> It seems that in the following you switched descriptions:

Shame on me :) Fixed.

> I have a little complain about how ExprEvalStep gets resvalue. resvalue is
> assigned in one place (within ExecEvalSubscriptingRefFetch(),
> ExecEvalSubscriptingRefAssign()), resnull is assigned in another place
> (within jsonb_subscript_fetch(), jsonb_subscript_assign()).

Hm...I'm afraid I don't get this. `resnull` is never assigned inside
`jsonb_subscript_fetch` or `jsonb_subscript_assign`, instead it's coming
from `ExecInterpExp` as `isnull` if I remember correctly. Are we talking
about
the same thing?

In this version of the patch I also improved NULL handling, you can see it
in
the tests.

Attachment Content-Type Size
0001-Base-implementation-of-subscripting-mechanism.patch application/octet-stream 168.0 KB
0002-Subscripting-for-array.patch application/octet-stream 12.9 KB
0003-Subscripting-for-jsonb.patch application/octet-stream 32.5 KB
0004-Subscripting-documentation.patch application/octet-stream 21.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-11-11 17:02:41 Re: Simplify ACL handling for large objects and removal of superuser() checks
Previous Message Fabrízio de Royes Mello 2017-11-11 12:35:35 Re: [PATCH] A hook for session start