Quick-and-dirty fix for recursive plpgsql functions, per bug report from
authorTom Lane
Fri, 21 Sep 2001 00:11:31 +0000 (00:11 +0000)
committerTom Lane
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);