On Wed, Oct 14, 2015 at 07:52:15PM -0400, Robert Haas wrote:
> On Tue, Oct 13, 2015 at 9:08 PM, Noah Misch <n...@leadboat.com> wrote:
> > On Mon, Oct 12, 2015 at 11:46:08AM -0400, Robert Haas wrote:
> >> Calls from SerializeParamList() need the same treatment as
> >> calls from copyParamList() because it, too, will try to evaluate every
> >> parameter in the list.

> > Like you, I don't expect bms_is_member() to be expensive relative to the 
> > task
> > at hand.  However, copyParamList() and SerializeParamList() copy non-dynamic
> > params without calling plpgsql_param_fetch().  Given the shared param list,
> > they will copy non-dynamic params the current query doesn't use.  That cost 
> > is
> > best avoided, not being well-bounded; consider the case of an unrelated
> > variable containing a TOAST pointer to a 1-GiB value.  One approach is to 
> > have
> > setup_param_list() copy the paramnos pointer to a new ParamListInfoData 
> > field:
> >
> >         Bitmapset  *paramMask;  /* if non-NULL, ignore params lacking a 
> > 1-bit */
> >
> > Test it directly in copyParamList() and SerializeParamList().  As a bonus,
> > that would allow use of the shared param list for more cases involving
> > cursors.  Furthermore, plpgsql_param_fetch() would never need to test
> > paramnos.  A more-general alternative is to have a distinct "paramIsUsed"
> > callback, but I don't know how one would exploit the extra generality.
> I'm anxious to minimize the number of things that must be fixed in
> order for a stable version of parallel query to exist in our master
> repository, and I fear that trying to improve ParamListInfo generally
> could take me fairly far afield.  How about adding a paramListCopyHook
> to ParamListInfoData?  SerializeParamList() would, if it found a
> parameter with !OidIsValid(prm->prmtype) && param->paramFetch != NULL,
> call this function, which would return a new ParamListInfo to be
> serialized in place of the original?

Tests of prm->prmtype and paramLI->paramFetch appear superfluous.  Given that
the paramListCopyHook callback would return a complete substitute
ParamListInfo, I wouldn't expect SerializeParamList() to examine the the
original paramLI->params at all.  If that's correct, the paramListCopyHook
design sounds fine.  However, its implementation will be more complex than
paramMask would have been.

> This wouldn't require any
> modification to the current plpgsql_param_fetch() at all, but the new
> function would steal its bms_is_member() test.  Furthermore, no user
> of ParamListInfo other than plpgsql needs to care at all -- which,
> with your proposals, they would.

To my knowledge, none of these approaches would compel existing users to care.
They would leave paramMask or paramListCopyHook NULL and get today's behavior.

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to