Re: Converting plpgsql to use DTYPE_REC for named composite types

Hi

2017-12-29 9:56 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

> Hi
>
> I'll stick this into the January commitfest, but I'd like to get it
>> reviewed and committed pretty soon, because there are follow-on patches
>> that need to get done in time for v11 --- in particular, we need to close
>> out the lack of plpgsql support for domains-over-composite.
>>
>>
> I didn't checked code - just I did some performance tests and I am
> thinking so performance is very good.
>
> Master's record type has 50% speed of row type in my test. Patched has +/-
> same speed.
>
> I see very small slowdown for row type .. about 3% but I think so it is
> acceptable - I tested some worst case.
>
> Unfortunately - it breaks and very breaks all plpgsql related extensions -
> pldebug, plprofiler, plpgsql_check. On second hand, there are only few
> extensions of this kind.
>
>
I checked the code:

Interesting part from test:

alter table mutable drop column f1;
alter table mutable add column f1 float8;
-- currently, this fails due to cached plan for "r.f1 + 1" expression
select sillyaddone(42);
ERROR: type of parameter 4 (double precision) does not match that when
preparing the plan (integer)
CONTEXT: PL/pgSQL function sillyaddone(integer) line 1 at RETURN

In this case, can we invalidate plan cache? It can decrease a risk of
runtime issues when tables are altered.

Because PLPGSQL_NSTYPE_ROW is removed, then "switch" statement is maybe
useless

if (ns != NULL && nnames == 2)
{
switch (ns->itemtype)
{
case PLPGSQL_NSTYPE_REC:
{
/*
* words 1/2 are a record name, so third word could
be
* a field in this record.
*/
PLpgSQL_rec *rec;
PLpgSQL_recfield *new;

rec = (PLpgSQL_rec *) (plpgsql_Datums[ns->itemno]);
new = plpgsql_build_recfield(rec, word3);

wdatum->datum = (PLpgSQL_datum *) new;
wdatum->ident = NULL;
wdatum->quoted = false; /* not used */
wdatum->idents = idents;
return true;
}

default:
break;
}
}
}

should be reduced

if (ns != NULL && nnames == 2 && ns->itemtype ==
PLPGSQL_NSTYPE_REC)
{
/*
* words 1/2 are a record name, so third word could
be
* a field in this record.
*/
PLpgSQL_rec *rec;
PLpgSQL_recfield *new;

rec = (PLpgSQL_rec *) (plpgsql_Datums[ns->itemno]);
new = plpgsql_build_recfield(rec, word3);

wdatum->datum = (PLpgSQL_datum *) new;
wdatum->ident = NULL;
wdatum->quoted = false; /* not used */
wdatum->idents = idents;
return true;
}

why is in exec_assign_value still case for PLPGSQL_DTYPE_ROW ?

Regards

Pavel

> Regards
>
> Pavel
>
>
> regards, tom lane
>>
>>
>