Horton

Re: [HACKERS] [PATCH] Generic type subscripting

Artur Zakirov <a.zakirov@postgrespro.ru>

View thread View on postgr.es
From: Arthur Zakirov <a.zakirov@postgrespro.ru>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Oleg Bartunov <obartunov@gmail.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Tom Lane <tgl@sss.pgh.pa.us>, David Steele <david@pgmasters.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2017-11-13T13:11:06Z
Lists: pgsql-hackers 
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

Commits

  1. Throw error when assigning jsonb scalar instead of a composite object

  2. Filling array gaps during jsonb subscripting

  3. Implementation of subscripting for jsonb

  4. Allow ALTER TYPE to update an existing type's typsubscript value.

  5. Allow subscripting of hstore values.

  6. Support subscripting of arbitrary types, not only arrays.

  7. jit: Reference function pointer types via llvmjit_types.c.

  8. Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.

  9. jit: Correct parameter type for generated expression evaluation functions.

  10. Renaming for new subscripting mechanism

  11. Fix assertion failure for SSL connections.

  12. Teach eval_const_expressions() to handle some more cases.