Avoid using a local FunctionCallInfoData struct in ExecMakeFunctionResult

and related routines.

We already had a redundant FunctionCallInfoData struct in FuncExprState,
but were using that copy only in set-returning-function cases, to avoid
keeping function evaluation state in the expression tree for the benefit
of plpgsql's "simple expression" logic.  But of course that didn't work
anyway.  Given the recent fixes in plpgsql there is no need to have two
separate behaviors here.  Getting rid of the local FunctionCallInfoData
structs should make things a little faster (because we don't need to do
InitFunctionCallInfoData each time), and it also makes for a noticeable
reduction in stack space consumption during recursive calls.

src/backend/executor/execQual.c

@@ -1204,6 +1204,10 @@ init_fcache(Oid foid, FuncExprState *fcache,
 	fmgr_info_cxt(foid, &(fcache->func), fcacheCxt);
 	fcache->func.fn_expr = (Node *) fcache->xprstate.expr;
 
+	/* Initialize the function call parameter struct as well */
+	InitFunctionCallInfoData(fcache->fcinfo_data, &(fcache->func),
+							 list_length(fcache->args), NULL, NULL);
+
 	/* If function returns set, prepare expected tuple descriptor */
 	if (fcache->func.fn_retset && needDescForSets)
 	{
@@ -1385,7 +1389,7 @@ ExecEvalFuncArgs(FunctionCallInfo fcinfo,
 		i++;
 	}
 
-	fcinfo->nargs = i;
+	Assert(i == fcinfo->nargs);
 
 	return argIsDone;
 }
@@ -1535,7 +1539,6 @@ ExecMakeFunctionResult(FuncExprState *fcache,
 {
 	List	   *arguments;
 	Datum		result;
-	FunctionCallInfoData fcinfo_data;
 	FunctionCallInfo fcinfo;
 	PgStat_FunctionCallUsage fcusage;
 	ReturnSetInfo rsinfo;		/* for functions returning sets */
@@ -1586,31 +1589,16 @@ restart:
 		Assert(!fcache->setArgsValid);
 	}
 
-	/*
-	 * For non-set-returning functions, we just use a local-variable
-	 * FunctionCallInfoData.  For set-returning functions we keep the callinfo
-	 * record in fcache->setArgs so that it can survive across multiple
-	 * value-per-call invocations.	(The reason we don't just do the latter
-	 * all the time is that plpgsql expects to be able to use simple
-	 * expression trees re-entrantly.  Which might not be a good idea, but the
-	 * penalty for not doing so is high.)
-	 */
-	if (fcache->func.fn_retset)
-		fcinfo = &fcache->setArgs;
-	else
-		fcinfo = &fcinfo_data;
-
 	/*
 	 * arguments is a list of expressions to evaluate before passing to the
 	 * function manager.  We skip the evaluation if it was already done in the
 	 * previous call (ie, we are continuing the evaluation of a set-valued
 	 * function).  Otherwise, collect the current argument values into fcinfo.
 	 */
+	fcinfo = &fcache->fcinfo_data;
 	arguments = fcache->args;
 	if (!fcache->setArgsValid)
 	{
-		/* Need to prep callinfo structure */
-		InitFunctionCallInfoData(*fcinfo, &(fcache->func), 0, NULL, NULL);
 		argDone = ExecEvalFuncArgs(fcinfo, arguments, econtext);
 		if (argDone == ExprEndResult)
 		{
@@ -1726,7 +1714,6 @@ restart:
 					if (fcache->func.fn_retset &&
 						*isDone == ExprMultipleResult)
 					{
-						Assert(fcinfo == &fcache->setArgs);
 						fcache->setHasSetArg = hasSetArg;
 						fcache->setArgsValid = true;
 						/* Register cleanup callback if we didn't already */
@@ -1856,7 +1843,7 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 {
 	ListCell   *arg;
 	Datum		result;
-	FunctionCallInfoData fcinfo;
+	FunctionCallInfo fcinfo;
 	PgStat_FunctionCallUsage fcusage;
 	int			i;
 
@@ -1867,20 +1854,19 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 		*isDone = ExprSingleResult;
 
 	/* inlined, simplified version of ExecEvalFuncArgs */
+	fcinfo = &fcache->fcinfo_data;
 	i = 0;
 	foreach(arg, fcache->args)
 	{
 		ExprState  *argstate = (ExprState *) lfirst(arg);
 
-		fcinfo.arg[i] = ExecEvalExpr(argstate,
-									 econtext,
-									 &fcinfo.argnull[i],
-									 NULL);
+		fcinfo->arg[i] = ExecEvalExpr(argstate,
+									  econtext,
+									  &fcinfo->argnull[i],
+									  NULL);
 		i++;
 	}
 
-	InitFunctionCallInfoData(fcinfo, &(fcache->func), i, NULL, NULL);
-
 	/*
 	 * If function is strict, and there are any NULL arguments, skip calling
 	 * the function and return NULL.
@@ -1889,7 +1875,7 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 	{
 		while (--i >= 0)
 		{
-			if (fcinfo.argnull[i])
+			if (fcinfo->argnull[i])
 			{
 				*isNull = true;
 				return (Datum) 0;
@@ -1897,11 +1883,11 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 		}
 	}
 
-	pgstat_init_function_usage(&fcinfo, &fcusage);
+	pgstat_init_function_usage(fcinfo, &fcusage);
 
-	/* fcinfo.isnull = false; */	/* handled by InitFunctionCallInfoData */
-	result = FunctionCallInvoke(&fcinfo);
-	*isNull = fcinfo.isnull;
+	fcinfo->isnull = false;
+	result = FunctionCallInvoke(fcinfo);
+	*isNull = fcinfo->isnull;
 
 	pgstat_end_function_usage(&fcusage, true);
 
@@ -1948,7 +1934,6 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 	 * resultinfo, but set it up anyway because we use some of the fields as
 	 * our own state variables.
 	 */
-	InitFunctionCallInfoData(fcinfo, NULL, 0, NULL, (Node *) &rsinfo);
 	rsinfo.type = T_ReturnSetInfo;
 	rsinfo.econtext = econtext;
 	rsinfo.expectedDesc = expectedDesc;
@@ -1992,6 +1977,9 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 						econtext->ecxt_per_query_memory, false);
 		}
 		returnsSet = fcache->func.fn_retset;
+		InitFunctionCallInfoData(fcinfo, &(fcache->func),
+								 list_length(fcache->args),
+								 NULL, (Node *) &rsinfo);
 
 		/*
 		 * Evaluate the function's argument list.
@@ -2001,7 +1989,6 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 		 * inner loop.	So do it in caller context.  Perhaps we should make a
 		 * separate context just to hold the evaluated arguments?
 		 */
-		fcinfo.flinfo = &(fcache->func);
 		argDone = ExecEvalFuncArgs(&fcinfo, fcache->args, econtext);
 		/* We don't allow sets in the arguments of the table function */
 		if (argDone != ExprSingleResult)
@@ -2029,6 +2016,7 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 	{
 		/* Treat funcexpr as a generic expression */
 		direct_function_call = false;
+		InitFunctionCallInfoData(fcinfo, NULL, 0, NULL, NULL);
 	}
 
 	/*
@@ -2312,9 +2300,8 @@ ExecEvalDistinct(FuncExprState *fcache,
 				 ExprDoneCond *isDone)
 {
 	Datum		result;
-	FunctionCallInfoData fcinfo;
+	FunctionCallInfo fcinfo;
 	ExprDoneCond argDone;
-	List	   *argList;
 
 	/* Set default values for result flags: non-null, not a set result */
 	*isNull = false;
@@ -2334,34 +2321,31 @@ ExecEvalDistinct(FuncExprState *fcache,
 	}
 
 	/*
-	 * extract info from fcache
+	 * Evaluate arguments
 	 */
-	argList = fcache->args;
-
-	/* Need to prep callinfo structure */
-	InitFunctionCallInfoData(fcinfo, &(fcache->func), 0, NULL, NULL);
-	argDone = ExecEvalFuncArgs(&fcinfo, argList, econtext);
+	fcinfo = &fcache->fcinfo_data;
+	argDone = ExecEvalFuncArgs(fcinfo, fcache->args, econtext);
 	if (argDone != ExprSingleResult)
 		ereport(ERROR,
 				(errcode(ERRCODE_DATATYPE_MISMATCH),
 				 errmsg("IS DISTINCT FROM does not support set arguments")));
-	Assert(fcinfo.nargs == 2);
+	Assert(fcinfo->nargs == 2);
 
-	if (fcinfo.argnull[0] && fcinfo.argnull[1])
+	if (fcinfo->argnull[0] && fcinfo->argnull[1])
 	{
 		/* Both NULL? Then is not distinct... */
 		result = BoolGetDatum(FALSE);
 	}
-	else if (fcinfo.argnull[0] || fcinfo.argnull[1])
+	else if (fcinfo->argnull[0] || fcinfo->argnull[1])
 	{
 		/* Only one is NULL? Then is distinct... */
 		result = BoolGetDatum(TRUE);
 	}
 	else
 	{
-		fcinfo.isnull = false;
-		result = FunctionCallInvoke(&fcinfo);
-		*isNull = fcinfo.isnull;
+		fcinfo->isnull = false;
+		result = FunctionCallInvoke(fcinfo);
+		*isNull = fcinfo->isnull;
 		/* Must invert result of "=" */
 		result = BoolGetDatum(!DatumGetBool(result));
 	}
@@ -2388,7 +2372,7 @@ ExecEvalScalarArrayOp(ScalarArrayOpExprState *sstate,
 	int			nitems;
 	Datum		result;
 	bool		resultnull;
-	FunctionCallInfoData fcinfo;
+	FunctionCallInfo fcinfo;
 	ExprDoneCond argDone;
 	int			i;
 	int16		typlen;
@@ -2413,26 +2397,28 @@ ExecEvalScalarArrayOp(ScalarArrayOpExprState *sstate,
 		Assert(!sstate->fxprstate.func.fn_retset);
 	}
 
-	/* Need to prep callinfo structure */
-	InitFunctionCallInfoData(fcinfo, &(sstate->fxprstate.func), 0, NULL, NULL);
-	argDone = ExecEvalFuncArgs(&fcinfo, sstate->fxprstate.args, econtext);
+	/*
+	 * Evaluate arguments
+	 */
+	fcinfo = &sstate->fxprstate.fcinfo_data;
+	argDone = ExecEvalFuncArgs(fcinfo, sstate->fxprstate.args, econtext);
 	if (argDone != ExprSingleResult)
 		ereport(ERROR,
 				(errcode(ERRCODE_DATATYPE_MISMATCH),
 			   errmsg("op ANY/ALL (array) does not support set arguments")));
-	Assert(fcinfo.nargs == 2);
+	Assert(fcinfo->nargs == 2);
 
 	/*
 	 * If the array is NULL then we return NULL --- it's not very meaningful
 	 * to do anything else, even if the operator isn't strict.
 	 */
-	if (fcinfo.argnull[1])
+	if (fcinfo->argnull[1])
 	{
 		*isNull = true;
 		return (Datum) 0;
 	}
 	/* Else okay to fetch and detoast the array */
-	arr = DatumGetArrayTypeP(fcinfo.arg[1]);
+	arr = DatumGetArrayTypeP(fcinfo->arg[1]);
 
 	/*
 	 * If the array is empty, we return either FALSE or TRUE per the useOr
@@ -2448,7 +2434,7 @@ ExecEvalScalarArrayOp(ScalarArrayOpExprState *sstate,
 	 * If the scalar is NULL, and the function is strict, return NULL; no
 	 * point in iterating the loop.
 	 */
-	if (fcinfo.argnull[0] && sstate->fxprstate.func.fn_strict)
+	if (fcinfo->argnull[0] && sstate->fxprstate.func.fn_strict)
 	{
 		*isNull = true;
 		return (Datum) 0;
@@ -2486,32 +2472,32 @@ ExecEvalScalarArrayOp(ScalarArrayOpExprState *sstate,
 		/* Get array element, checking for NULL */
 		if (bitmap && (*bitmap & bitmask) == 0)
 		{
-			fcinfo.arg[1] = (Datum) 0;
-			fcinfo.argnull[1] = true;
+			fcinfo->arg[1] = (Datum) 0;
+			fcinfo->argnull[1] = true;
 		}
 		else
 		{
 			elt = fetch_att(s, typbyval, typlen);
 			s = att_addlength_pointer(s, typlen, s);
 			s = (char *) att_align_nominal(s, typalign);
-			fcinfo.arg[1] = elt;
-			fcinfo.argnull[1] = false;
+			fcinfo->arg[1] = elt;
+			fcinfo->argnull[1] = false;
 		}
 
 		/* Call comparison function */
-		if (fcinfo.argnull[1] && sstate->fxprstate.func.fn_strict)
+		if (fcinfo->argnull[1] && sstate->fxprstate.func.fn_strict)
 		{
-			fcinfo.isnull = true;
+			fcinfo->isnull = true;
 			thisresult = (Datum) 0;
 		}
 		else
 		{
-			fcinfo.isnull = false;
-			thisresult = FunctionCallInvoke(&fcinfo);
+			fcinfo->isnull = false;
+			thisresult = FunctionCallInvoke(fcinfo);
 		}
 
 		/* Combine results per OR or AND semantics */
-		if (fcinfo.isnull)
+		if (fcinfo->isnull)
 			resultnull = true;
 		else if (useOr)
 		{
@@ -3526,9 +3512,8 @@ ExecEvalNullIf(FuncExprState *nullIfExpr,
 			   bool *isNull, ExprDoneCond *isDone)
 {
 	Datum		result;
-	FunctionCallInfoData fcinfo;
+	FunctionCallInfo fcinfo;
 	ExprDoneCond argDone;
-	List	   *argList;
 
 	if (isDone)
 		*isDone = ExprSingleResult;
@@ -3546,26 +3531,23 @@ ExecEvalNullIf(FuncExprState *nullIfExpr,
 	}
 
 	/*
-	 * extract info from nullIfExpr
+	 * Evaluate arguments
 	 */
-	argList = nullIfExpr->args;
-
-	/* Need to prep callinfo structure */
-	InitFunctionCallInfoData(fcinfo, &(nullIfExpr->func), 0, NULL, NULL);
-	argDone = ExecEvalFuncArgs(&fcinfo, argList, econtext);
+	fcinfo = &nullIfExpr->fcinfo_data;
+	argDone = ExecEvalFuncArgs(fcinfo, nullIfExpr->args, econtext);
 	if (argDone != ExprSingleResult)
 		ereport(ERROR,
 				(errcode(ERRCODE_DATATYPE_MISMATCH),
 				 errmsg("NULLIF does not support set arguments")));
-	Assert(fcinfo.nargs == 2);
+	Assert(fcinfo->nargs == 2);
 
 	/* if either argument is NULL they can't be equal */
-	if (!fcinfo.argnull[0] && !fcinfo.argnull[1])
+	if (!fcinfo->argnull[0] && !fcinfo->argnull[1])
 	{
-		fcinfo.isnull = false;
-		result = FunctionCallInvoke(&fcinfo);
+		fcinfo->isnull = false;
+		result = FunctionCallInvoke(fcinfo);
 		/* if the arguments are equal return null */
-		if (!fcinfo.isnull && DatumGetBool(result))
+		if (!fcinfo->isnull && DatumGetBool(result))
 		{
 			*isNull = true;
 			return (Datum) 0;
@@ -3573,8 +3555,8 @@ ExecEvalNullIf(FuncExprState *nullIfExpr,
 	}
 
 	/* else return first argument */
-	*isNull = fcinfo.argnull[0];
-	return fcinfo.arg[0];
+	*isNull = fcinfo->argnull[0];
+	return fcinfo->arg[0];
 }
 
 /* ----------------------------------------------------------------

src/include/nodes/execnodes.h

@@ -623,13 +623,11 @@ typedef struct FuncExprState
 										 * NULL */
 
 	/*
-	 * We need to store argument values across calls when evaluating a SRF
-	 * that uses value-per-call mode.
-	 *
-	 * setArgsValid is true when we are evaluating a set-valued function and
-	 * we are in the middle of a call series; we want to pass the same
-	 * argument values to the function again (and again, until it returns
-	 * ExprEndResult).
+	 * setArgsValid is true when we are evaluating a set-returning function
+	 * that uses value-per-call mode and we are in the middle of a call
+	 * series; we want to pass the same argument values to the function again
+	 * (and again, until it returns ExprEndResult).  This indicates that
+	 * fcinfo_data already contains valid argument data.
 	 */
 	bool		setArgsValid;
 
@@ -649,10 +647,11 @@ typedef struct FuncExprState
 	bool		shutdown_reg;	/* a shutdown callback is registered */
 
 	/*
-	 * Current argument data for a set-valued function; contains valid data
-	 * only if setArgsValid is true.
+	 * Call parameter structure for the function.  This has been initialized
+	 * (by InitFunctionCallInfoData) if func.fn_oid is valid.  It also saves
+	 * argument values between calls, when setArgsValid is true.
 	 */
-	FunctionCallInfoData setArgs;
+	FunctionCallInfoData fcinfo_data;
 } FuncExprState;
 
 /* ----------------