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
