Re: Letting plpgsql in on the fun with the new expression eval stuff - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Letting plpgsql in on the fun with the new expression eval stuff
Date
Msg-id 4249.1513811175@sss.pgh.pa.us
Whole thread Raw
In response to Re: Letting plpgsql in on the fun with the new expression eval stuff  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Letting plpgsql in on the fun with the new expression eval stuff  (Robert Haas <robertmhaas@gmail.com>)
List 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 = ¶mInfo->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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: domain cast in parameterized vs. non-parameterized query
Next
From: Haisheng Yuan
Date:
Subject: Re: Bitmap table scan cost per page formula