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

1 files changed, 60 insertions, 3 deletions

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 = &paramLI->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.
  */