On Mon, Oct 12, 2015 at 11:46:08AM -0400, Robert Haas wrote:
> plpgsql_param_fetch() assumes that it can detect whether it's being
> called from copyParamList() by checking whether params !=
> estate->paramLI. I don't know why this works, but I do know that this
It works because PL/pgSQL creates an unshared list whenever copyParamList() is
forthcoming. (This in turn relies on intimate knowledge of how the rest of
the system processes param lists.) The comments at setup_param_list() and
setup_unshared_param_list() are most pertinent.
> test fails to detect the case where it's being called from
> SerializeParamList(), which causes failures in exec_eval_datum() as
> predicted. Calls from SerializeParamList() need the same treatment as
> calls from copyParamList() because it, too, will try to evaluate every
> parameter in the list. Here, I've taken the approach of making that
> check unconditional, which seems to work, but I'm not sure if some
> other approach would be better, such as adding an additional Boolean
> (or enum context?) argument to ParamFetchHook. I *think* that
> skipping this check is merely a performance optimization rather than
> anything that affects correctness, and bms_is_member() is pretty
> cheap, so perhaps the way I've done it is OK.
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.
