Re: [HACKERS] [PATCH] Generic type subscripting - Mailing list pgsql-hackers

From Dmitry Dolgov
Subject Re: [HACKERS] [PATCH] Generic type subscripting
Date
Msg-id CA+q6zcX5_meFnpnQj=wY5iyNXs-i_x+KpJ8eyZGnvn3CaUS2YQ@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] [PATCH] Generic type subscripting  (Arthur Zakirov <a.zakirov@postgrespro.ru>)
Responses Re: [HACKERS] [PATCH] Generic type subscripting
List pgsql-hackers
> On 8 November 2017 at 17:25, Arthur Zakirov <a.zakirov@postgrespro.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

pgsql-hackers by date:

Previous
From: Fabrízio de Royes Mello
Date:
Subject: Re: [HACKERS] [PATCH] A hook for session start
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] Simplify ACL handling for large objects and removal of superuser() checks