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