Re: Parallel Seq Scan - Mailing list pgsql-hackers

On Wed, Oct 14, 2015 at 07:52:15PM -0400, Robert Haas wrote:
> On Tue, Oct 13, 2015 at 9:08 PM, Noah Misch <noah@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.