Re: [HACKERS] [PATCH] Generic type subscripting

On Sat, Nov 11, 2017 at 04:34:31PM +0100, Dmitry Dolgov wrote:
> > >
> > > 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.

Great. I think users will know how to use isAssignment now.

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

No, I meant ExprEvalStep struct. For example, within ExecEvalSubscriptingRefFetch() you assign *op->resvalue but *op->resnull is leaved as unchanged:

> ExecEvalSubscriptingRefFetch(ExprState *state, ExprEvalStep *op)
> ...
> *op->resvalue = FunctionCall2(op->d.sbsref.eval_finfo,
> PointerGetDatum(*op->resvalue),
> PointerGetDatum(op));

For jsonb *op->resnull is changed within jsonb_subscript_fetch() for arrays within array_subscript_fetch() (which are called by ExecEvalSubscriptingRefFetch()):

> return jsonb_get_element(DatumGetJsonbP(containerSource),
> sbstate->upper,
> sbstate->numupper,
> step->resnull, /* step->resnull is changed within jsonb_get_element() */
> false);

It is not critical, but it may be good to change them in one place.

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

New tests passed.

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company