diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index fb950fdfd14b63b3c8f67abd0643877764ed3a9f..ea32b5ab1d2ffc44bc4dd3f033583262cdc3a7eb 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.87 2001/06/19 22:39:11 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.88 2001/09/21 00:11:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -54,9 +54,8 @@ static Datum ExecEvalOper(Expr *opClause, ExprContext *econtext, bool *isNull, ExprDoneCond *isDone); static Datum ExecEvalFunc(Expr *funcClause, ExprContext *econtext, bool *isNull, ExprDoneCond *isDone); -static ExprDoneCond ExecEvalFuncArgs(FunctionCachePtr fcache, - List *argList, - ExprContext *econtext); +static ExprDoneCond ExecEvalFuncArgs(FunctionCallInfo fcinfo, + List *argList, ExprContext *econtext); static Datum ExecEvalNot(Expr *notclause, ExprContext *econtext, bool *isNull); static Datum ExecEvalAnd(Expr *andExpr, ExprContext *econtext, bool *isNull); static Datum ExecEvalOr(Expr *orExpr, ExprContext *econtext, bool *isNull); @@ -600,7 +599,7 @@ GetAttributeByName(TupleTableSlot *slot, char *attname, bool *isNull) * Evaluate arguments for a function. */ static ExprDoneCond -ExecEvalFuncArgs(FunctionCachePtr fcache, +ExecEvalFuncArgs(FunctionCallInfo fcinfo, List *argList, ExprContext *econtext) { @@ -615,14 +614,13 @@ ExecEvalFuncArgs(FunctionCachePtr fcache, { ExprDoneCond thisArgIsDone; - fcache->fcinfo.arg[i] = ExecEvalExpr((Node *) lfirst(arg), - econtext, - &fcache->fcinfo.argnull[i], - &thisArgIsDone); + fcinfo->arg[i] = ExecEvalExpr((Node *) lfirst(arg), + econtext, + &fcinfo->argnull[i], + &thisArgIsDone); if (thisArgIsDone != ExprSingleResult) { - /* * We allow only one argument to have a set value; we'd need * much more complexity to keep track of multiple set @@ -631,12 +629,13 @@ ExecEvalFuncArgs(FunctionCachePtr fcache, */ if (argIsDone != ExprSingleResult) elog(ERROR, "Functions and operators can take only one set argument"); - fcache->hasSetArg = true; argIsDone = thisArgIsDone; } i++; } + fcinfo->nargs = i; + return argIsDone; } @@ -656,7 +655,10 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, ExprDoneCond *isDone) { Datum result; + FunctionCallInfoData fcinfo; + ReturnSetInfo rsinfo; /* for functions returning sets */ ExprDoneCond argDone; + bool hasSetArg; int i; /* @@ -664,11 +666,14 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, * 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 fcache->fcinfo. + * values into fcinfo. */ - if (fcache->fcinfo.nargs > 0 && !fcache->argsValid) + if (!fcache->setArgsValid) { - argDone = ExecEvalFuncArgs(fcache, arguments, econtext); + /* Need to prep callinfo structure */ + MemSet(&fcinfo, 0, sizeof(fcinfo)); + fcinfo.flinfo = &(fcache->func); + argDone = ExecEvalFuncArgs(&fcinfo, arguments, econtext); if (argDone == ExprEndResult) { /* input is an empty set, so return an empty set. */ @@ -679,15 +684,33 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, elog(ERROR, "Set-valued function called in context that cannot accept a set"); return (Datum) 0; } + hasSetArg = (argDone != ExprSingleResult); + } + else + { + /* Copy callinfo from previous evaluation */ + memcpy(&fcinfo, &fcache->setArgs, sizeof(fcinfo)); + hasSetArg = fcache->setHasSetArg; + /* Reset flag (we may set it again below) */ + fcache->setArgsValid = false; + } + + /* + * If function returns set, prepare a resultinfo node for + * communication + */ + if (fcache->func.fn_retset) + { + fcinfo.resultinfo = (Node *) &rsinfo; + rsinfo.type = T_ReturnSetInfo; } /* * now return the value gotten by calling the function manager, * passing the function the evaluated parameter values. */ - if (fcache->func.fn_retset || fcache->hasSetArg) + if (fcache->func.fn_retset || hasSetArg) { - /* * We need to return a set result. Complain if caller not ready * to accept one. @@ -705,7 +728,6 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, */ for (;;) { - /* * If function is strict, and there are any NULL arguments, * skip calling the function (at least for this set of args). @@ -714,9 +736,9 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, if (fcache->func.fn_strict) { - for (i = 0; i < fcache->fcinfo.nargs; i++) + for (i = 0; i < fcinfo.nargs; i++) { - if (fcache->fcinfo.argnull[i]) + if (fcinfo.argnull[i]) { callit = false; break; @@ -726,11 +748,11 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, if (callit) { - fcache->fcinfo.isnull = false; - fcache->rsinfo.isDone = ExprSingleResult; - result = FunctionCallInvoke(&fcache->fcinfo); - *isNull = fcache->fcinfo.isnull; - *isDone = fcache->rsinfo.isDone; + fcinfo.isnull = false; + rsinfo.isDone = ExprSingleResult; + result = FunctionCallInvoke(&fcinfo); + *isNull = fcinfo.isnull; + *isDone = rsinfo.isDone; } else { @@ -741,14 +763,17 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, if (*isDone != ExprEndResult) { - /* * Got a result from current argument. If function itself - * returns set, flag that we want to reuse current - * argument values on next call. + * returns set, save the current argument values to re-use + * on the next call. */ if (fcache->func.fn_retset) - fcache->argsValid = true; + { + memcpy(&fcache->setArgs, &fcinfo, sizeof(fcinfo)); + fcache->setHasSetArg = hasSetArg; + fcache->setArgsValid = true; + } /* * Make sure we say we are returning a set, even if the @@ -759,22 +784,15 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, } /* Else, done with this argument */ - fcache->argsValid = false; - - if (!fcache->hasSetArg) + if (!hasSetArg) break; /* input not a set, so done */ /* Re-eval args to get the next element of the input set */ - argDone = ExecEvalFuncArgs(fcache, arguments, econtext); + argDone = ExecEvalFuncArgs(&fcinfo, arguments, econtext); if (argDone != ExprMultipleResult) { - - /* - * End of arguments, so reset the hasSetArg flag and say - * "Done" - */ - fcache->hasSetArg = false; + /* End of argument set, so we're done. */ *isNull = true; *isDone = ExprEndResult; result = (Datum) 0; @@ -789,7 +807,6 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, } else { - /* * Non-set case: much easier. * @@ -798,18 +815,18 @@ ExecMakeFunctionResult(FunctionCachePtr fcache, */ if (fcache->func.fn_strict) { - for (i = 0; i < fcache->fcinfo.nargs; i++) + for (i = 0; i < fcinfo.nargs; i++) { - if (fcache->fcinfo.argnull[i]) + if (fcinfo.argnull[i]) { *isNull = true; return (Datum) 0; } } } - fcache->fcinfo.isnull = false; - result = FunctionCallInvoke(&fcache->fcinfo); - *isNull = fcache->fcinfo.isnull; + fcinfo.isnull = false; + result = FunctionCallInvoke(&fcinfo); + *isNull = fcinfo.isnull; } return result; diff --git a/src/backend/utils/cache/fcache.c b/src/backend/utils/cache/fcache.c index 91bea5cfc71e26d53206ec5e752222b196f3c876..bb1ac36f3efdf3715e438074fd7d2ca288804866 100644 --- a/src/backend/utils/cache/fcache.c +++ b/src/backend/utils/cache/fcache.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/cache/Attic/fcache.c,v 1.39 2001/03/22 03:59:57 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/cache/Attic/fcache.c,v 1.40 2001/09/21 00:11:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -17,11 +17,8 @@ #include "utils/fcache.h" -/*----------------------------------------------------------------- - * +/* * Build a 'FunctionCache' struct given the PG_PROC oid. - * - *----------------------------------------------------------------- */ FunctionCachePtr init_fcache(Oid foid, int nargs, MemoryContext fcacheCxt) @@ -29,6 +26,10 @@ init_fcache(Oid foid, int nargs, MemoryContext fcacheCxt) MemoryContext oldcontext; FunctionCachePtr retval; + /* Safety check (should never fail, as parser should check sooner) */ + if (nargs > FUNC_MAX_ARGS) + elog(ERROR, "init_fcache: too many arguments"); + /* Switch to a context long-lived enough for the fcache entry */ oldcontext = MemoryContextSwitchTo(fcacheCxt); @@ -38,25 +39,8 @@ init_fcache(Oid foid, int nargs, MemoryContext fcacheCxt) /* Set up the primary fmgr lookup information */ fmgr_info(foid, &(retval->func)); - /* Initialize unvarying fields of per-call info block */ - retval->fcinfo.flinfo = &(retval->func); - retval->fcinfo.nargs = nargs; - - if (nargs > FUNC_MAX_ARGS) - elog(ERROR, "init_fcache: too many arguments"); - - /* - * If function returns set, prepare a resultinfo node for - * communication - */ - if (retval->func.fn_retset) - { - retval->fcinfo.resultinfo = (Node *) &(retval->rsinfo); - retval->rsinfo.type = T_ReturnSetInfo; - } - - retval->argsValid = false; - retval->hasSetArg = false; + /* Initialize additional info */ + retval->setArgsValid = false; MemoryContextSwitchTo(oldcontext); diff --git a/src/include/utils/fcache.h b/src/include/utils/fcache.h index bb081a04748d0154d7ee3d543ee318d899fd7038..7d94590feba702f73bf6485b955da2ed5eefaa9b 100644 --- a/src/include/utils/fcache.h +++ b/src/include/utils/fcache.h @@ -11,7 +11,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: fcache.h,v 1.16 2001/03/22 04:01:12 momjian Exp $ + * $Id: fcache.h,v 1.17 2001/09/21 00:11:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -25,46 +25,42 @@ * A FunctionCache record is built for all functions regardless of language. * * We store the fmgr lookup info to avoid recomputing it on each call. - * We also store a prebuilt FunctionCallInfo struct. When evaluating a - * function-returning-set, fcinfo holds the argument values across calls - * so that we need not re-evaluate the arguments for each call. Even for - * non-set functions, fcinfo saves a few cycles per call by allowing us to - * avoid redundant setup of its fields. + * + * We also need to store argument values across calls when evaluating a + * function-returning-set. This is pretty ugly (and not re-entrant); + * current-evaluation info should be somewhere in the econtext, not in + * the querytree. As it stands, a function-returning-set can't safely be + * recursive, at least not if it's in plpgsql which will try to re-use + * the querytree at multiple execution nesting levels. FIXME someday. */ typedef struct FunctionCache { - /* * Function manager's lookup info for the target function. */ FmgrInfo func; /* - * Per-call info for calling the target function. Unvarying fields - * are set up by init_fcache(). Argument values are filled in as - * needed. - */ - FunctionCallInfoData fcinfo; - - /* - * "Resultinfo" node --- used only if target function returns a set. - */ - ReturnSetInfo rsinfo; - - /* - * argsValid 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 + * 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). */ - bool argsValid; /* TRUE if fcinfo contains valid arguments */ + bool setArgsValid; /* - * hasSetArg is true if we found a set-valued argument to the + * Flag to remember whether we found a set-valued argument to the * function. This causes the function result to be a set as well. + * Valid only when setArgsValid is true. + */ + bool setHasSetArg; /* some argument returns a set */ + + /* + * Current argument data for a set-valued function; contains valid + * data only if setArgsValid is true. */ - bool hasSetArg; /* some argument returns a set */ + FunctionCallInfoData setArgs; } FunctionCache; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index ac6ba3253dafd7fdd8a642ce8d1bd89b446419cb..cb40912a42edfca0abd18f455edcf1666d45e0ab 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -1515,3 +1515,22 @@ insert into IFace values ('IF', 'notthere', 'eth0', ''); ERROR: system "notthere" does not exist insert into IFace values ('IF', 'orion', 'ethernet_interface_name_too_long', ''); ERROR: IFace slotname "IF.orion.ethernet_interface_name_too_long" too long (20 char max) +-- +-- Test recursion, per bug report 7-Sep-01 +-- +CREATE FUNCTION recursion_test(int,int) RETURNS text AS ' +DECLARE rslt text; +BEGIN + IF $1 <= 0 THEN + rslt = CAST($2 AS TEXT); + ELSE + rslt = CAST($1 AS TEXT) || '','' || recursion_test($1 - 1, $2); + END IF; + RETURN rslt; +END;' LANGUAGE 'plpgsql'; +SELECT recursion_test(4,3); + recursion_test +---------------- + 4,3,2,1,3 +(1 row) + diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 5bfda232981ce7ccc1014926acf71d29b9296bda..6ce6e364e69269050b2113a789a96570d5d7db25 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -1399,3 +1399,18 @@ delete from HSlot; insert into IFace values ('IF', 'notthere', 'eth0', ''); insert into IFace values ('IF', 'orion', 'ethernet_interface_name_too_long', ''); +-- +-- Test recursion, per bug report 7-Sep-01 +-- +CREATE FUNCTION recursion_test(int,int) RETURNS text AS ' +DECLARE rslt text; +BEGIN + IF $1 <= 0 THEN + rslt = CAST($2 AS TEXT); + ELSE + rslt = CAST($1 AS TEXT) || '','' || recursion_test($1 - 1, $2); + END IF; + RETURN rslt; +END;' LANGUAGE 'plpgsql'; + +SELECT recursion_test(4,3);