Re: Letting plpgsql in on the fun with the new expression eval stuff

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Letting plpgsql in on the fun with the new expression eval stuff
Date: 2017-12-20 23:06:15
Message-ID: 4249.1513811175@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> * Redesign the API for the ParamListInfo paramFetch hook so that the
> ParamExternData array can be entirely virtual. Typical access to
> the info about a PARAM_EXTERN Param now looks like
>
> if (paramInfo->paramFetch != NULL)
> prm = paramInfo->paramFetch(paramInfo, paramId, ...);
> else
> prm = &paramInfo->params[paramId - 1];
>
> so that if paramFetch is defined, the core code no longer touches
> the params[] array at all, and it doesn't have to be there.

I forgot to mention that I dithered about changing the params field
from a variable-length array to a pointer, ie,

- ParamExternData params[FLEXIBLE_ARRAY_MEMBER];
+ ParamExternData *params;

Then we could set the pointer to NULL in cases where no physical
array is provided, which would be a good thing in terms of helping
to catch code that hasn't been updated to the new convention.
However, this would force less-than-trivial changes in every place
that creates a ParamListInfo. For instance, copyParamList might
change from

size = offsetof(ParamListInfoData, params) +
from->numParams * sizeof(ParamExternData);

retval = (ParamListInfo) palloc(size);
... fill retval ...

to

size = MAXALIGN(sizeof(ParamListInfoData)) +
from->numParams * sizeof(ParamExternData);

retval = (ParamListInfo) palloc(size);
retval->params = (ParamExternData *)
((char *) retval + MAXALIGN(sizeof(ParamListInfoData)));
... fill rest of retval ...

That seemed like a pain in the rear, and easy to get wrong
(although it could be a lot simpler if you didn't mind doing
two pallocs instead of one).

Now there aren't that many places in the core code that do this,
so it wouldn't be very hard to fix them, but I'm concerned about
the possible impact on extension modules. Creating param lists
seems like something that a lot of things would have code for.

Anyway, I left it as-is, but I'm willing to make the change if
people feel the other way is better.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haisheng Yuan 2017-12-20 23:13:10 Re: Bitmap table scan cost per page formula
Previous Message Robert Haas 2017-12-20 22:45:37 Re: vacuum vs heap_update_tuple() and multixactids