Horton

Re: [PATCH] Generic type subscripting

Dmitry Dolgov <9erthalion6@gmail.com>

View thread View on postgr.es
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Arthur Zakirov <a.zakirov@postgrespro.ru>
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-07T20:00:43Z
Lists: pgsql-hackers

Attachments

> On 31 October 2017 at 16:05, Arthur Zakirov <a.zakirov@postgrespro.ru>
wrote:
> Documentation is compiled. But there are warnings about end-tags. Now it
is necessary to have full named end-tags

Fixed, thanks for noticing.

> I think it is necessary to check Oids of subscripting_parse,
subscripting_assign, subscripting_fetch. Maybe within TypeCreate().

Yes, I agree. I implemented it in a way that all subscripting-related
functions
must be provided if `subscripting_parse` is there - in this case if you
want to
prevent assign or fetch, you can just do it inside a corresponding function
and
it allows to provide a custom message about that.

> > +Datum
> > +custom_subscripting_parse(PG_FUNCTION_ARGS)
> > +{
> > +     bool                            isAssignment = PG_GETARG_BOOL(0);
>
> 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`.

> > +     scratch->d.sbsref.eval_finfo = eval_finfo;
> > +     scratch->d.sbsref.nested_finfo = nested_finfo;
> > +
> As I mentioned earlier we need assigning eval_finfo and nested_finfo only
for EEOP_SBSREF_OLD, EEOP_SBSREF_ASSIGN and EEOP_SBSREF_FETCH steps.
> Also they should be assigned before calling ExprEvalPushStep(), not
after. Otherwise some bugs may appear in future.

I'm really confused about this one. Can you tell me the exact line numbers?
Because if I remove any of these lines "blindly", tests are failing.

> > -             ArrayRef   *aref = makeNode(ArrayRef);
> > +             NodeTag sbstag = nodeTag(src_expr);
> > +             Size nodeSize = sizeof(SubscriptingRef);
> > +             SubscriptingRef *sbsref = (SubscriptingRef *)
newNode(nodeSize, sbstag);
>
> Is there necessity to use newNode() instead using makeNode(). The
previous code was shorter.

Good catch! It was a leftover from the version when I had two different
nodes
for subscripting.

> There is no changes in execnodes.h except removed line. So I think
execnodes.h could be removed from the patch.

Fixed.

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.