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