Redesign the plancache mechanism for more flexibility and efficiency.

Rewrite plancache.c so that a "cached plan" (which is rather a misnomer
at this point) can support generation of custom, parameter-value-dependent
plans, and can make an intelligent choice between using custom plans and
the traditional generic-plan approach.  The specific choice algorithm
implemented here can probably be improved in future, but this commit is
all about getting the mechanism in place, not the policy.

In addition, restructure the API to greatly reduce the amount of extraneous
data copying needed.  The main compromise needed to make that possible was
to split the initial creation of a CachedPlanSource into two steps.  It's
worth noting in particular that SPI_saveplan is now deprecated in favor of
SPI_keepplan, which accomplishes the same end result with zero data
copying, and no need to then spend even more cycles throwing away the
original SPIPlan.  The risk of long-term memory leaks while manipulating
SPIPlans has also been greatly reduced.  Most of this improvement is based
on use of the recently-added MemoryContextSetParent primitive.

src/pl/plperl/plperl.c

@@ -165,7 +165,7 @@ typedef struct plperl_call_data
 typedef struct plperl_query_desc
 {
 	char		qname[24];
-	void	   *plan;
+	SPIPlanPtr	plan;
 	int			nargs;
 	Oid		   *argtypes;
 	FmgrInfo   *arginfuncs;
@@ -2951,7 +2951,7 @@ plperl_spi_query(char *query)
 
 	PG_TRY();
 	{
-		void	   *plan;
+		SPIPlanPtr	plan;
 		Portal		portal;
 
 		/* Make sure the query is validly encoded */
@@ -3118,7 +3118,7 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
 	plperl_query_desc *qdesc;
 	plperl_query_entry *hash_entry;
 	bool		found;
-	void	   *plan;
+	SPIPlanPtr	plan;
 	int			i;
 
 	MemoryContext oldcontext = CurrentMemoryContext;
@@ -3182,13 +3182,9 @@ plperl_spi_prepare(char *query, int argc, SV **argv)
 		 * Save the plan into permanent memory (right now it's in the
 		 * SPI procCxt, which will go away at function end).
 		 ************************************************************/
-		qdesc->plan = SPI_saveplan(plan);
-		if (qdesc->plan == NULL)
-			elog(ERROR, "SPI_saveplan() failed: %s",
-				 SPI_result_code_string(SPI_result));
-
-		/* Release the procCxt copy to avoid within-function memory leak */
-		SPI_freeplan(plan);
+		if (SPI_keepplan(plan))
+			elog(ERROR, "SPI_keepplan() failed");
+		qdesc->plan = plan;
 
 		/* Commit the inner transaction, return to outer xact context */
 		ReleaseCurrentSubTransaction();
@@ -3516,7 +3512,7 @@ plperl_spi_query_prepared(char *query, int argc, SV **argv)
 void
 plperl_spi_freeplan(char *query)
 {
-	void	   *plan;
+	SPIPlanPtr	plan;
 	plperl_query_desc *qdesc;
 	plperl_query_entry *hash_entry;
 

src/pl/plpgsql/src/pl_exec.c

@@ -142,6 +142,7 @@ static void exec_prepare_plan(PLpgSQL_execstate *estate,
 				  PLpgSQL_expr *expr, int cursorOptions);
 static bool exec_simple_check_node(Node *node);
 static void exec_simple_check_plan(PLpgSQL_expr *expr);
+static void exec_simple_recheck_plan(PLpgSQL_expr *expr, CachedPlan *cplan);
 static bool exec_eval_simple_expr(PLpgSQL_execstate *estate,
 					  PLpgSQL_expr *expr,
 					  Datum *result,
@@ -2020,8 +2021,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
 		exec_prepare_plan(estate, query, curvar->cursor_options);
 
 	/*
-	 * Set up ParamListInfo (note this is only carrying a hook function, not
-	 * any actual data values, at this point)
+	 * Set up ParamListInfo (hook function and possibly data values)
 	 */
 	paramLI = setup_param_list(estate, query);
 
@@ -2991,8 +2991,10 @@ exec_prepare_plan(PLpgSQL_execstate *estate,
 					 expr->query, SPI_result_code_string(SPI_result));
 		}
 	}
-	expr->plan = SPI_saveplan(plan);
-	SPI_freeplan(plan);
+	SPI_keepplan(plan);
+	expr->plan = plan;
+
+	/* Check to see if it's a simple expression */
 	exec_simple_check_plan(expr);
 }
 
@@ -3010,6 +3012,11 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
 	int			rc;
 	PLpgSQL_expr *expr = stmt->sqlstmt;
 
+	/*
+	 * Set up ParamListInfo (hook function and possibly data values)
+	 */
+	paramLI = setup_param_list(estate, expr);
+
 	/*
 	 * On the first call for this statement generate the plan, and detect
 	 * whether the statement is INSERT/UPDATE/DELETE
@@ -3025,28 +3032,23 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
 			CachedPlanSource *plansource = (CachedPlanSource *) lfirst(l);
 			ListCell   *l2;
 
-			foreach(l2, plansource->plan->stmt_list)
+			Assert(plansource->is_valid);
+			foreach(l2, plansource->query_list)
 			{
-				PlannedStmt *p = (PlannedStmt *) lfirst(l2);
+				Query  *q = (Query *) lfirst(l2);
 
-				if (IsA(p, PlannedStmt) &&
-					p->canSetTag)
+				Assert(IsA(q, Query));
+				if (q->canSetTag)
 				{
-					if (p->commandType == CMD_INSERT ||
-						p->commandType == CMD_UPDATE ||
-						p->commandType == CMD_DELETE)
+					if (q->commandType == CMD_INSERT ||
+						q->commandType == CMD_UPDATE ||
+						q->commandType == CMD_DELETE)
 						stmt->mod_stmt = true;
 				}
 			}
 		}
 	}
 
-	/*
-	 * Set up ParamListInfo (note this is only carrying a hook function, not
-	 * any actual data values, at this point)
-	 */
-	paramLI = setup_param_list(estate, expr);
-
 	/*
 	 * If we have INTO, then we only need one row back ... but if we have INTO
 	 * STRICT, ask for two rows, so that we can verify the statement returns
@@ -3520,8 +3522,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
 	}
 
 	/*
-	 * Set up ParamListInfo (note this is only carrying a hook function, not
-	 * any actual data values, at this point)
+	 * Set up ParamListInfo (hook function and possibly data values)
 	 */
 	paramLI = setup_param_list(estate, query);
 
@@ -4613,8 +4614,7 @@ exec_run_select(PLpgSQL_execstate *estate,
 		exec_prepare_plan(estate, expr, 0);
 
 	/*
-	 * Set up ParamListInfo (note this is only carrying a hook function, not
-	 * any actual data values, at this point)
+	 * Set up ParamListInfo (hook function and possibly data values)
 	 */
 	paramLI = setup_param_list(estate, expr);
 
@@ -4833,11 +4833,10 @@ loop_exit:
  *
  * It is possible though unlikely for a simple expression to become non-simple
  * (consider for example redefining a trivial view).  We must handle that for
- * correctness; fortunately it's normally inexpensive to do
- * RevalidateCachedPlan on a simple expression.  We do not consider the other
- * direction (non-simple expression becoming simple) because we'll still give
- * correct results if that happens, and it's unlikely to be worth the cycles
- * to check.
+ * correctness; fortunately it's normally inexpensive to do GetCachedPlan on a
+ * simple expression.  We do not consider the other direction (non-simple
+ * expression becoming simple) because we'll still give correct results if
+ * that happens, and it's unlikely to be worth the cycles to check.
  *
  * Note: if pass-by-reference, the result is in the eval_econtext's
  * temporary memory context.  It will be freed when exec_eval_cleanup
@@ -4873,17 +4872,21 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
 
 	/*
 	 * Revalidate cached plan, so that we will notice if it became stale. (We
-	 * also need to hold a refcount while using the plan.)	Note that even if
-	 * replanning occurs, the length of plancache_list can't change, since it
-	 * is a property of the raw parsetree generated from the query text.
+	 * need to hold a refcount while using the plan, anyway.)  Note that even
+	 * if replanning occurs, the length of plancache_list can't change, since
+	 * it is a property of the raw parsetree generated from the query text.
 	 */
 	Assert(list_length(expr->plan->plancache_list) == 1);
 	plansource = (CachedPlanSource *) linitial(expr->plan->plancache_list);
-	cplan = RevalidateCachedPlan(plansource, true);
+
+	/* Get the generic plan for the query */
+	cplan = GetCachedPlan(plansource, NULL, true);
+	Assert(cplan == plansource->gplan);
+
 	if (cplan->generation != expr->expr_simple_generation)
 	{
 		/* It got replanned ... is it still simple? */
-		exec_simple_check_plan(expr);
+		exec_simple_recheck_plan(expr, cplan);
 		if (expr->expr_simple_expr == NULL)
 		{
 			/* Ooops, release refcount and fail */
@@ -4900,7 +4903,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
 	/*
 	 * Prepare the expression for execution, if it's not been done already in
 	 * the current transaction.  (This will be forced to happen if we called
-	 * exec_simple_check_plan above.)
+	 * exec_simple_recheck_plan above.)
 	 */
 	if (expr->expr_simple_lxid != curlxid)
 	{
@@ -4931,9 +4934,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
 	 * need to free it explicitly, since it will go away at the next reset of
 	 * that context.
 	 *
-	 * XXX think about avoiding repeated palloc's for param lists?  It should
-	 * be possible --- this routine isn't re-entrant anymore.
-	 *
 	 * Just for paranoia's sake, save and restore the prior value of
 	 * estate->cur_expr, which setup_param_list() sets.
 	 */
@@ -4982,10 +4982,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
 /*
  * Create a ParamListInfo to pass to SPI
  *
- * The ParamListInfo array is initially all zeroes, in particular the
- * ptype values are all InvalidOid.  This causes the executor to call the
- * paramFetch hook each time it wants a value.	We thus evaluate only the
- * parameters actually demanded.
+ * We fill in the values for any expression parameters that are plain
+ * PLpgSQL_var datums; these are cheap and safe to evaluate, and by setting
+ * them with PARAM_FLAG_CONST flags, we allow the planner to use those values
+ * in custom plans.  However, parameters that are not plain PLpgSQL_vars
+ * should not be evaluated here, because they could throw errors (for example
+ * "no such record field") and we do not want that to happen in a part of
+ * the expression that might never be evaluated at runtime.  To handle those
+ * parameters, we set up a paramFetch hook for the executor to call when it
+ * wants a not-presupplied value.
  *
  * The result is a locally palloc'd array that should be pfree'd after use;
  * but note it can be NULL.
@@ -4997,21 +5002,42 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
 
 	/*
 	 * Could we re-use these arrays instead of palloc'ing a new one each time?
-	 * However, we'd have to zero the array each time anyway, since new values
-	 * might have been assigned to the variables.
+	 * However, we'd have to re-fill the array each time anyway, since new
+	 * values might have been assigned to the variables.
 	 */
 	if (estate->ndatums > 0)
 	{
-		/* sizeof(ParamListInfoData) includes the first array element */
+		Bitmapset  *tmpset;
+		int			dno;
+
 		paramLI = (ParamListInfo)
-			palloc0(sizeof(ParamListInfoData) +
-					(estate->ndatums - 1) * sizeof(ParamExternData));
+			palloc0(offsetof(ParamListInfoData, params) +
+					estate->ndatums * sizeof(ParamExternData));
 		paramLI->paramFetch = plpgsql_param_fetch;
 		paramLI->paramFetchArg = (void *) estate;
 		paramLI->parserSetup = (ParserSetupHook) plpgsql_parser_setup;
 		paramLI->parserSetupArg = (void *) expr;
 		paramLI->numParams = estate->ndatums;
 
+		/* Instantiate values for "safe" parameters of the expression */
+		tmpset = bms_copy(expr->paramnos);
+		while ((dno = bms_first_member(tmpset)) >= 0)
+		{
+			PLpgSQL_datum *datum = estate->datums[dno];
+
+			if (datum->dtype == PLPGSQL_DTYPE_VAR)
+			{
+				PLpgSQL_var *var = (PLpgSQL_var *) datum;
+				ParamExternData *prm = &paramLI->params[dno];
+
+				prm->value = var->value;
+				prm->isnull = var->isnull;
+				prm->pflags = PARAM_FLAG_CONST;
+				prm->ptype = var->datatype->typoid;
+			}
+		}
+		bms_free(tmpset);
+
 		/*
 		 * Set up link to active expr where the hook functions can find it.
 		 * Callers must save and restore cur_expr if there is any chance that
@@ -5628,30 +5654,113 @@ static void
 exec_simple_check_plan(PLpgSQL_expr *expr)
 {
 	CachedPlanSource *plansource;
-	PlannedStmt *stmt;
-	Plan	   *plan;
-	TargetEntry *tle;
+	Query	   *query;
+	CachedPlan *cplan;
 
 	/*
 	 * Initialize to "not simple", and remember the plan generation number we
-	 * last checked.  (If the query produces more or less than one parsetree
+	 * last checked.  (If we don't get as far as obtaining a plan to check,
 	 * we just leave expr_simple_generation set to 0.)
 	 */
 	expr->expr_simple_expr = NULL;
 	expr->expr_simple_generation = 0;
 
 	/*
-	 * 1. We can only evaluate queries that resulted in one single execution
-	 * plan
+	 * We can only test queries that resulted in exactly one CachedPlanSource
 	 */
 	if (list_length(expr->plan->plancache_list) != 1)
 		return;
 	plansource = (CachedPlanSource *) linitial(expr->plan->plancache_list);
-	expr->expr_simple_generation = plansource->generation;
-	if (list_length(plansource->plan->stmt_list) != 1)
+
+	/*
+	 * Do some checking on the analyzed-and-rewritten form of the query.
+	 * These checks are basically redundant with the tests in
+	 * exec_simple_recheck_plan, but the point is to avoid building a plan if
+	 * possible.  Since this function is only
+	 * called immediately after creating the CachedPlanSource, we need not
+	 * worry about the query being stale.
+	 */
+
+	/*
+	 * 1. There must be one single querytree.
+	 */
+	if (list_length(plansource->query_list) != 1)
+		return;
+	query = (Query *) linitial(plansource->query_list);
+
+	/*
+	 * 2. It must be a plain SELECT query without any input tables
+	 */
+	if (!IsA(query, Query))
+		return;
+	if (query->commandType != CMD_SELECT || query->intoClause)
+		return;
+	if (query->rtable != NIL)
 		return;
 
-	stmt = (PlannedStmt *) linitial(plansource->plan->stmt_list);
+	/*
+	 * 3. Can't have any subplans, aggregates, qual clauses either
+	 */
+	if (query->hasAggs ||
+		query->hasWindowFuncs ||
+		query->hasSubLinks ||
+		query->hasForUpdate ||
+		query->cteList ||
+		query->jointree->quals ||
+		query->groupClause ||
+		query->havingQual ||
+		query->windowClause ||
+		query->distinctClause ||
+		query->sortClause ||
+		query->limitOffset ||
+		query->limitCount ||
+		query->setOperations)
+		return;
+
+	/*
+	 * 4. The query must have a single attribute as result
+	 */
+	if (list_length(query->targetList) != 1)
+		return;
+
+	/*
+	 * OK, it seems worth constructing a plan for more careful checking.
+	 */
+
+	/* Get the generic plan for the query */
+	cplan = GetCachedPlan(plansource, NULL, true);
+	Assert(cplan == plansource->gplan);
+
+	/* Share the remaining work with recheck code path */
+	exec_simple_recheck_plan(expr, cplan);
+
+	/* Release our plan refcount */
+	ReleaseCachedPlan(cplan, true);
+}
+
+/*
+ * exec_simple_recheck_plan --- check for simple plan once we have CachedPlan
+ */
+static void
+exec_simple_recheck_plan(PLpgSQL_expr *expr, CachedPlan *cplan)
+{
+	PlannedStmt *stmt;
+	Plan	   *plan;
+	TargetEntry *tle;
+
+	/*
+	 * Initialize to "not simple", and remember the plan generation number we
+	 * last checked.
+	 */
+	expr->expr_simple_expr = NULL;
+	expr->expr_simple_generation = cplan->generation;
+
+	/*
+	 * 1. There must be one single plantree
+	 */
+	if (list_length(cplan->stmt_list) != 1)
+		return;
+	stmt = (PlannedStmt *) linitial(cplan->stmt_list);
 
 	/*
 	 * 2. It must be a RESULT plan --> no scan's required

src/pl/plpython/plpython.c

@@ -287,7 +287,7 @@ typedef struct PLySubtransactionData
 typedef struct PLyPlanObject
 {
 	PyObject_HEAD
-	void	   *plan;			/* return of an SPI_saveplan */
+	SPIPlanPtr	plan;
 	int			nargs;
 	Oid		   *types;
 	Datum	   *values;
@@ -3327,7 +3327,6 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
 	PyObject   *list = NULL;
 	PyObject   *volatile optr = NULL;
 	char	   *query;
-	void	   *tmpplan;
 	volatile MemoryContext oldcontext;
 	volatile ResourceOwner oldowner;
 	volatile int nargs;
@@ -3431,12 +3430,8 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
 				 SPI_result_code_string(SPI_result));
 
 		/* transfer plan from procCxt to topCxt */
-		tmpplan = plan->plan;
-		plan->plan = SPI_saveplan(tmpplan);
-		SPI_freeplan(tmpplan);
-		if (plan->plan == NULL)
-			elog(ERROR, "SPI_saveplan failed: %s",
-				 SPI_result_code_string(SPI_result));
+		if (SPI_keepplan(plan->plan))
+			elog(ERROR, "SPI_keepplan failed");
 
 		/* Commit the inner transaction, return to outer xact context */
 		ReleaseCurrentSubTransaction();

src/pl/tcl/pltcl.c

@@ -128,7 +128,7 @@ typedef struct pltcl_proc_desc
 typedef struct pltcl_query_desc
 {
 	char		qname[20];
-	void	   *plan;
+	SPIPlanPtr	plan;
 	int			nargs;
 	Oid		   *argtypes;
 	FmgrInfo   *arginfuncs;
@@ -2024,7 +2024,7 @@ pltcl_process_SPI_result(Tcl_Interp *interp,
  * pltcl_SPI_prepare()		- Builtin support for prepared plans
  *				  The Tcl command SPI_prepare
  *				  always saves the plan using
- *				  SPI_saveplan and returns a key for
+ *				  SPI_keepplan and returns a key for
  *				  access. There is no chance to prepare
  *				  and not save the plan currently.
  **********************************************************************/
@@ -2035,7 +2035,6 @@ pltcl_SPI_prepare(ClientData cdata, Tcl_Interp *interp,
 	int			nargs;
 	CONST84 char **args;
 	pltcl_query_desc *qdesc;
-	void	   *plan;
 	int			i;
 	Tcl_HashEntry *hashent;
 	int			hashnew;
@@ -2103,22 +2102,18 @@ pltcl_SPI_prepare(ClientData cdata, Tcl_Interp *interp,
 		 * Prepare the plan and check for errors
 		 ************************************************************/
 		UTF_BEGIN;
-		plan = SPI_prepare(UTF_U2E(argv[1]), nargs, qdesc->argtypes);
+		qdesc->plan = SPI_prepare(UTF_U2E(argv[1]), nargs, qdesc->argtypes);
 		UTF_END;
 
-		if (plan == NULL)
+		if (qdesc->plan == NULL)
 			elog(ERROR, "SPI_prepare() failed");
 
 		/************************************************************
 		 * Save the plan into permanent memory (right now it's in the
 		 * SPI procCxt, which will go away at function end).
 		 ************************************************************/
-		qdesc->plan = SPI_saveplan(plan);
-		if (qdesc->plan == NULL)
-			elog(ERROR, "SPI_saveplan() failed");
-
-		/* Release the procCxt copy to avoid within-function memory leak */
-		SPI_freeplan(plan);
+		if (SPI_keepplan(qdesc->plan))
+			elog(ERROR, "SPI_keepplan() failed");
 
 		pltcl_subtrans_commit(oldcontext, oldowner);
 	}