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: http://www.postgresql.org/mailpref/pgsql-hackers
