From 2f48ede080f42b97b594fb14102c82ca1001b80c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 12 Jun 2020 12:14:32 -0400 Subject: Avoid using a cursor in plpgsql's RETURN QUERY statement. plpgsql has always executed the query given in a RETURN QUERY command by opening it as a cursor and then fetching a few rows at a time, which it turns around and dumps into the function's result tuplestore. The point of this was to keep from blowing out memory with an oversized SPITupleTable result (note that while a tuplestore can spill tuples to disk, SPITupleTable cannot). However, it's rather inefficient, both because of extra data copying and because of executor entry/exit overhead. In recent versions, a new performance problem has emerged: use of a cursor prevents use of a parallel plan for the executed query. We can improve matters by skipping use of a cursor and having the executor push result tuples directly into the function's result tuplestore. However, a moderate amount of new infrastructure is needed to make that idea work: * We can use the existing tstoreReceiver.c DestReceiver code to funnel executor output to the tuplestore, but it has to be extended to support plpgsql's requirement for possibly applying a tuple conversion map. * SPI needs to be extended to allow use of a caller-supplied DestReceiver instead of its usual receiver that puts tuples into a SPITupleTable. Two new API calls are needed to handle both the RETURN QUERY and RETURN QUERY EXECUTE cases. I also felt that I didn't want these new API calls to use the legacy method of specifying query parameter values with "char" null flags (the old ' '/'n' convention); rather they should accept ParamListInfo objects containing the parameter type and value info. This required a bit of additional new infrastructure since we didn't yet have any parse analysis callback that would interpret $N parameter symbols according to type data supplied in a ParamListInfo. There seems to be no harm in letting makeParamList install that callback by default, rather than leaving a new ParamListInfo's parserSetup hook as NULL. (Indeed, as of HEAD, I couldn't find anyplace that was using the parserSetup field at all; plpgsql was using parserSetupArg for its own purposes, but parserSetup seemed to be write-only.) We can actually get plpgsql out of the business of using legacy null flags altogether, and using ParamListInfo instead of its ad-hoc PreparedParamsData structure; but this requires inventing one more SPI API call that can replace SPI_cursor_open_with_args. That seems worth doing, though. SPI_execute_with_args and SPI_cursor_open_with_args are now unused anywhere in the core PG distribution. Perhaps someday we could deprecate/remove them. But cleaning up the crufty bits of the SPI API is a task for a different patch. Per bug #16040 from Jeremy Smith. This is unfortunately too invasive to consider back-patching. Patch by me; thanks to Hamid Akhtar for review. Discussion: https://postgr.es/m/16040-eaacad11fecfb198@postgresql.org --- src/backend/nodes/params.c | 63 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 3 deletions(-) (limited to 'src/backend/nodes/params.c') diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c index ed2ee6a975a..1719119fc28 100644 --- a/src/backend/nodes/params.c +++ b/src/backend/nodes/params.c @@ -17,19 +17,27 @@ #include "access/xact.h" #include "mb/stringinfo_mb.h" -#include "nodes/bitmapset.h" #include "nodes/params.h" +#include "parser/parse_node.h" #include "storage/shmem.h" #include "utils/datum.h" #include "utils/lsyscache.h" #include "utils/memutils.h" +static void paramlist_parser_setup(ParseState *pstate, void *arg); +static Node *paramlist_param_ref(ParseState *pstate, ParamRef *pref); + + /* * Allocate and initialize a new ParamListInfo structure. * * To make a new structure for the "dynamic" way (with hooks), pass 0 for * numParams and set numParams manually. + * + * A default parserSetup function is supplied automatically. Callers may + * override it if they choose. (Note that most use-cases for ParamListInfos + * will never use the parserSetup function anyway.) */ ParamListInfo makeParamList(int numParams) @@ -45,8 +53,8 @@ makeParamList(int numParams) retval->paramFetchArg = NULL; retval->paramCompile = NULL; retval->paramCompileArg = NULL; - retval->parserSetup = NULL; - retval->parserSetupArg = NULL; + retval->parserSetup = paramlist_parser_setup; + retval->parserSetupArg = (void *) retval; retval->paramValuesStr = NULL; retval->numParams = numParams; @@ -102,6 +110,55 @@ copyParamList(ParamListInfo from) return retval; } + +/* + * Set up to parse a query containing references to parameters + * sourced from a ParamListInfo. + */ +static void +paramlist_parser_setup(ParseState *pstate, void *arg) +{ + pstate->p_paramref_hook = paramlist_param_ref; + /* no need to use p_coerce_param_hook */ + pstate->p_ref_hook_state = arg; +} + +/* + * Transform a ParamRef using parameter type data from a ParamListInfo. + */ +static Node * +paramlist_param_ref(ParseState *pstate, ParamRef *pref) +{ + ParamListInfo paramLI = (ParamListInfo) pstate->p_ref_hook_state; + int paramno = pref->number; + ParamExternData *prm; + ParamExternData prmdata; + Param *param; + + /* check parameter number is valid */ + if (paramno <= 0 || paramno > paramLI->numParams) + return NULL; + + /* give hook a chance in case parameter is dynamic */ + if (paramLI->paramFetch != NULL) + prm = paramLI->paramFetch(paramLI, paramno, false, &prmdata); + else + prm = ¶mLI->params[paramno - 1]; + + if (!OidIsValid(prm->ptype)) + return NULL; + + param = makeNode(Param); + param->paramkind = PARAM_EXTERN; + param->paramid = paramno; + param->paramtype = prm->ptype; + param->paramtypmod = -1; + param->paramcollid = get_typcollation(param->paramtype); + param->location = pref->location; + + return (Node *) param; +} + /* * Estimate the amount of space required to serialize a ParamListInfo. */ -- cgit v1.2.3