Quick-and-dirty fix for recursive plpgsql functions, per bug report from
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 21 Sep 2001 00:11:31 +0000 (00:11 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 21 Sep 2001 00:11:31 +0000 (00:11 +0000)
Frank Miles 7-Sep-01.  This is really just sticking a finger in the dike.
Frank's case works now, but we still couldn't support a recursive function
returning a set.  Really need to restructure querytrees and execution
state so that the querytree is *read only*.  We've run into this over and
over and over again ... it has to happen sometime soon.

src/backend/executor/execQual.c
src/backend/utils/cache/fcache.c
src/include/utils/fcache.h
src/test/regress/expected/plpgsql.out
src/test/regress/sql/plpgsql.sql

index fb950fdfd14b63b3c8f67abd0643877764ed3a9f..ea32b5ab1d2ffc44bc4dd3f033583262cdc3a7eb 100644 (file)
@@ -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;
index 91bea5cfc71e26d53206ec5e752222b196f3c876..bb1ac36f3efdf3715e438074fd7d2ca288804866 100644 (file)
@@ -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 $
  *
  *-------------------------------------------------------------------------
  */
 #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);
 
index bb081a04748d0154d7ee3d543ee318d899fd7038..7d94590feba702f73bf6485b955da2ed5eefaa9b 100644 (file)
@@ -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 $
  *
  *-------------------------------------------------------------------------
  */
  * 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;
 
 
index ac6ba3253dafd7fdd8a642ce8d1bd89b446419cb..cb40912a42edfca0abd18f455edcf1666d45e0ab 100644 (file)
@@ -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)
+
index 5bfda232981ce7ccc1014926acf71d29b9296bda..6ce6e364e69269050b2113a789a96570d5d7db25 100644 (file)
@@ -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);