On Wed, Nov 15, 2017 at 11:02:59PM +0100, Dmitry Dolgov wrote:
> On the second thought, no, looks like I'm wrong and it should be like this.
> The
> reason is that any `fetch` function should be in form
>
> (container, internal) -> extracted value
>
> which means that we need to return an extracted value (for jsonb it's a
> `jsonb`,
> for array it's an `anyelement`). But at the same time in general case we can
> figure out if the result is null only inside a `fetch` function,
> (`jsonb_get_element` for jsonb or whatever it may be for a custom data type)
> because it returns Datum. So the only way to return this information is by
> reference through the `internal` argument. To summarize, If as you said it's
> not that critical, I would suggest to leave it as it is.
Actually it is not only way to return isnull information. You can also return it using pointer to a boolean argument.
*fetch() functions also doesn't need in ExprEvalStep struct, you can pass SubscriptingRefState struct instead.
I mean the following code:
ExecEvalSubscriptingRefFetch(ExprState *state, ExprEvalStep *op)
{ ... *op->resvalue = FunctionCall2(op->d.sbsref.eval_finfo,
PointerGetDatum(*op->resvalue), PointerGetDatum(op->d.sbsref.state),
PointerGetDatum(op->resnull));
}
Datum
jsonb_subscript_fetch(PG_FUNCTION_ARGS)
{ Datum containerSource = PG_GETARG_DATUM(0); SubscriptingRefState *state = (SubscriptingRefState *)
PG_GETARG_POINTER(1); bool *isNull = (bool *) PG_GETARG_POINTER(2);
return jsonb_get_element(DatumGetJsonbP(containerSource), state->upper,
state->numupper, isNull, false);
}
> To summarize, If as you said it's
> not that critical, I would suggest to leave it as it is.
Yes, I just wanted to share an opinion how to improve the code. I thought that the current approach may confuse
programmers,who will implement subscribting.
Also you can see extractValue() function of GIN [1]. It returns if values is null in same way.
1 - https://www.postgresql.org/docs/current/static/gin-extensibility.html
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
