[Fwd: SETOF input parameters (was Re: [HACKERS] proposal: array utility functions phase 1)] - Mailing list pgsql-patches

Joe Conway wrote:
> Tom Lane wrote:
>
>> It seems like somehow we need a level of FROM/WHERE producing some base
>> rows, and then a set of table function calls to apply to each of the
>> base rows, and then another level of WHERE to filter the results of the
>> function calls (in particular to provide join conditions to identify
>> which rows to match up in the function outputs).  I don't see any way to
>> do this without inventing new SELECT clauses out of whole cloth
>> ... unless SQL99's WITH clause helps, but I don't think it does ...
>
>
> Well, maybe this is a start. It allows a table function's input
> parameter to be declared with setof. The changes involved primarily:
>
> 1) a big loop in ExecMakeTableFunctionResult so that functions with set
> returning arguments get called for each row of the argument,
>   and
> 2) aways initializing the tuplestore in ExecMakeTableFunctionResult and
> passing that to the function, even when SFRM_Materialize mode is used.
>
> The result looks like:
>
> create table foot(f1 text, f2 text);
> insert into foot values('a','b');
> insert into foot values('c','d');
> insert into foot values('e','f');
>
> create or replace function test2() returns setof foot as 'select * from
> foot order by 1 asc' language 'sql';
> create or replace function test(setof foot) returns foot as 'select
> $1.f1, $1.f2' language 'sql';
>
> regression=# select * from test(test2());
>  f1 | f2
> ----+----
>  a  | b
>  c  | d
>  e  | f
> (3 rows)
>
> I know it doesn't solve all the issues discussed, but is it a step
> forward? Suggestions?
>

Patch updated (again) to apply cleanly against cvs. Compiles clean and passes
all regression tests. Any feedback? If not, please apply.

Thanks,

Joe


Index: contrib/tablefunc/tablefunc.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/tablefunc/tablefunc.c,v
retrieving revision 1.11
diff -c -r1.11 tablefunc.c
*** contrib/tablefunc/tablefunc.c    23 Nov 2002 01:54:09 -0000    1.11
--- contrib/tablefunc/tablefunc.c    16 Dec 2002 21:21:47 -0000
***************
*** 53,59 ****
            int max_depth,
            bool show_branch,
            MemoryContext per_query_ctx,
!           AttInMetadata *attinmeta);
  static Tuplestorestate *build_tuplestore_recursively(char *key_fld,
                               char *parent_key_fld,
                               char *relname,
--- 53,60 ----
            int max_depth,
            bool show_branch,
            MemoryContext per_query_ctx,
!           AttInMetadata *attinmeta,
!           Tuplestorestate *tupstore);
  static Tuplestorestate *build_tuplestore_recursively(char *key_fld,
                               char *parent_key_fld,
                               char *relname,
***************
*** 641,646 ****
--- 642,648 ----
      char       *branch_delim = NULL;
      bool        show_branch = false;
      ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+     Tuplestorestate *tupstore;
      TupleDesc    tupdesc;
      AttInMetadata *attinmeta;
      MemoryContext per_query_ctx;
***************
*** 673,678 ****
--- 675,681 ----
               "allowed in this context");

      /* OK, go to work */
+     tupstore = rsinfo->setResult;
      rsinfo->returnMode = SFRM_Materialize;
      rsinfo->setResult = connectby(relname,
                                    key_fld,
***************
*** 682,688 ****
                                    max_depth,
                                    show_branch,
                                    per_query_ctx,
!                                   attinmeta);
      rsinfo->setDesc = tupdesc;

      MemoryContextSwitchTo(oldcontext);
--- 685,692 ----
                                    max_depth,
                                    show_branch,
                                    per_query_ctx,
!                                   attinmeta,
!                                   tupstore);
      rsinfo->setDesc = tupdesc;

      MemoryContextSwitchTo(oldcontext);
***************
*** 709,732 ****
            int max_depth,
            bool show_branch,
            MemoryContext per_query_ctx,
!           AttInMetadata *attinmeta)
  {
-     Tuplestorestate *tupstore = NULL;
      int            ret;
-     MemoryContext oldcontext;

      /* Connect to SPI manager */
      if ((ret = SPI_connect()) < 0)
          elog(ERROR, "connectby: SPI_connect returned %d", ret);

-     /* switch to longer term context to create the tuple store */
-     oldcontext = MemoryContextSwitchTo(per_query_ctx);
-
-     /* initialize our tuplestore */
-     tupstore = tuplestore_begin_heap(true, SortMem);
-
-     MemoryContextSwitchTo(oldcontext);
-
      /* now go get the whole tree */
      tupstore = build_tuplestore_recursively(key_fld,
                                              parent_key_fld,
--- 713,727 ----
            int max_depth,
            bool show_branch,
            MemoryContext per_query_ctx,
!           AttInMetadata *attinmeta,
!           Tuplestorestate *tupstore)
  {
      int            ret;

      /* Connect to SPI manager */
      if ((ret = SPI_connect()) < 0)
          elog(ERROR, "connectby: SPI_connect returned %d", ret);

      /* now go get the whole tree */
      tupstore = build_tuplestore_recursively(key_fld,
                                              parent_key_fld,
***************
*** 742,751 ****
                                              tupstore);

      SPI_finish();
-
-     oldcontext = MemoryContextSwitchTo(per_query_ctx);
-     tuplestore_donestoring(tupstore);
-     MemoryContextSwitchTo(oldcontext);

      return tupstore;
  }
--- 737,742 ----
Index: src/backend/commands/functioncmds.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/commands/functioncmds.c,v
retrieving revision 1.24
diff -c -r1.24 functioncmds.c
*** src/backend/commands/functioncmds.c    1 Nov 2002 19:19:58 -0000    1.24
--- src/backend/commands/functioncmds.c    16 Dec 2002 21:21:47 -0000
***************
*** 160,168 ****
                   TypeNameToString(t));
          }

-         if (t->setof)
-             elog(ERROR, "Functions cannot accept set arguments");
-
          parameterTypes[parameterCount++] = toid;
      }

--- 160,165 ----
Index: src/backend/executor/execQual.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/executor/execQual.c,v
retrieving revision 1.121
diff -c -r1.121 execQual.c
*** src/backend/executor/execQual.c    15 Dec 2002 16:17:45 -0000    1.121
--- src/backend/executor/execQual.c    16 Dec 2002 21:21:47 -0000
***************
*** 874,879 ****
--- 874,880 ----
      bool        direct_function_call;
      bool        first_time = true;
      bool        returnsTuple = false;
+     FuncExprState *fcache = (FuncExprState *) funcexpr;

      /*
       * Normally the passed expression tree will be a FuncExprState, since the
***************
*** 888,896 ****
      if (funcexpr && IsA(funcexpr, FuncExprState) &&
          IsA(funcexpr->expr, FuncExpr))
      {
-         FuncExprState *fcache = (FuncExprState *) funcexpr;
-         ExprDoneCond argDone;
-
          /*
           * This path is similar to ExecMakeFunctionResult.
           */
--- 889,894 ----
***************
*** 906,943 ****
              init_fcache(func->funcid, fcache, econtext->ecxt_per_query_memory);
          }

-         /*
-          * Evaluate the function's argument list.
-          *
-          * Note: ideally, we'd do this in the per-tuple context, but then the
-          * argument values would disappear when we reset the context in the
-          * inner loop.    So do it in caller context.  Perhaps we should make a
-          * separate context just to hold the evaluated arguments?
-          */
          MemSet(&fcinfo, 0, sizeof(fcinfo));
          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)
-             elog(ERROR, "Set-valued function called in context that cannot accept a set");
-
-         /*
-          * If function is strict, and there are any NULL arguments, skip
-          * calling the function and return NULL (actually an empty set).
-          */
-         if (fcache->func.fn_strict)
-         {
-             int            i;
-
-             for (i = 0; i < fcinfo.nargs; i++)
-             {
-                 if (fcinfo.argnull[i])
-                 {
-                     *returnDesc = NULL;
-                     return NULL;
-                 }
-             }
-         }
      }
      else
      {
--- 904,911 ----
***************
*** 964,1104 ****
      rsinfo.setResult = NULL;
      rsinfo.setDesc = NULL;

!     /*
!      * Switch to short-lived context for calling the function or expression.
!      */
!     callerContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
!
!     /*
!      * Loop to handle the ValuePerCall protocol (which is also the same
!      * behavior needed in the generic ExecEvalExpr path).
!      */
!     for (;;)
      {
!         Datum        result;
!         HeapTuple    tuple;
!
!         /*
!          * reset per-tuple memory context before each call of the
!          * function or expression. This cleans up any local memory the
!          * function may leak when called.
!          */
!         ResetExprContext(econtext);

-         /* Call the function or expression one time */
          if (direct_function_call)
          {
!             fcinfo.isnull = false;
!             rsinfo.isDone = ExprSingleResult;
!             result = FunctionCallInvoke(&fcinfo);
!         }
!         else
!         {
!             result = ExecEvalExpr(funcexpr, econtext,
!                                   &fcinfo.isnull, &rsinfo.isDone);
          }

!         /* Which protocol does function want to use? */
!         if (rsinfo.returnMode == SFRM_ValuePerCall)
          {
              /*
!              * Check for end of result set.
!              *
!              * Note: if function returns an empty set, we don't build a
!              * tupdesc or tuplestore (since we can't get a tupdesc in the
!              * function-returning-tuple case)
               */
!             if (rsinfo.isDone == ExprEndResult)
!                 break;

              /*
!              * If first time through, build tupdesc and tuplestore for
!              * result
               */
              if (first_time)
              {
                  oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
!                 if (funcrettype == RECORDOID ||
!                     get_typtype(funcrettype) == 'c')
                  {
-                     /*
-                      * Composite type, so function should have returned a
-                      * TupleTableSlot; use its descriptor
-                      */
                      slot = (TupleTableSlot *) DatumGetPointer(result);
                      if (fcinfo.isnull ||
                          !slot ||
                          !IsA(slot, TupleTableSlot) ||
!                         !slot->ttc_tupleDescriptor)
                          elog(ERROR, "ExecMakeTableFunctionResult: Invalid result from function returning tuple");
!                     tupdesc = CreateTupleDescCopy(slot->ttc_tupleDescriptor);
!                     returnsTuple = true;
                  }
                  else
                  {
!                     /*
!                      * Scalar type, so make a single-column descriptor
!                      */
!                     tupdesc = CreateTemplateTupleDesc(1, false);
!                     TupleDescInitEntry(tupdesc,
!                                        (AttrNumber) 1,
!                                        "column",
!                                        funcrettype,
!                                        -1,
!                                        0,
!                                        false);
                  }
!                 tupstore = tuplestore_begin_heap(true,    /* randomAccess */
!                                                  SortMem);
                  MemoryContextSwitchTo(oldcontext);
-                 rsinfo.setResult = tupstore;
-                 rsinfo.setDesc = tupdesc;
-             }

!             /*
!              * Store current resultset item.
!              */
!             if (returnsTuple)
!             {
!                 slot = (TupleTableSlot *) DatumGetPointer(result);
!                 if (fcinfo.isnull ||
!                     !slot ||
!                     !IsA(slot, TupleTableSlot) ||
!                     TupIsNull(slot))
!                     elog(ERROR, "ExecMakeTableFunctionResult: Invalid result from function returning tuple");
!                 tuple = slot->val;
              }
!             else
              {
!                 char        nullflag;
!
!                 nullflag = fcinfo.isnull ? 'n' : ' ';
!                 tuple = heap_formtuple(tupdesc, &result, &nullflag);
!             }
!
!             oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
!             tuplestore_puttuple(tupstore, tuple);
!             MemoryContextSwitchTo(oldcontext);

!             /*
!              * Are we done?
!              */
!             if (rsinfo.isDone != ExprMultipleResult)
                  break;
          }
-         else if (rsinfo.returnMode == SFRM_Materialize)
-         {
-             /* check we're on the same page as the function author */
-             if (!first_time || rsinfo.isDone != ExprSingleResult)
-                 elog(ERROR, "ExecMakeTableFunctionResult: Materialize-mode protocol not followed");
-             /* Done evaluating the set result */
-             break;
-         }
-         else
-             elog(ERROR, "ExecMakeTableFunctionResult: unknown returnMode %d",
-                  (int) rsinfo.returnMode);

!         first_time = false;
      }

      /* If we have a locally-created tupstore, close it up */
--- 932,1121 ----
      rsinfo.setResult = NULL;
      rsinfo.setDesc = NULL;

!     callerContext = CurrentMemoryContext;
!     while(1)
      {
!         ExprDoneCond argDone = ExprSingleResult; /* until proven otherwise */

          if (direct_function_call)
          {
!             /*
!              * Evaluate the function's argument list.
!              *
!              * Note: ideally, we'd do this in the per-tuple context, but then the
!              * argument values would disappear when we reset the context in the
!              * inner loop.    So do it in caller context.  Perhaps we should make a
!              * separate context just to hold the evaluated arguments?
!              */
!             MemoryContextSwitchTo(callerContext);
!             argDone = ExecEvalFuncArgs(&fcinfo, fcache->args, econtext);
!             if (argDone == ExprEndResult)
!                 break;
!
!             /*
!              * If function is strict, and there are any NULL arguments, skip
!              * calling the function and return NULL (actually an empty set).
!              */
!             if (fcache->func.fn_strict)
!             {
!                 int            i;
!
!                 for (i = 0; i < fcinfo.nargs; i++)
!                 {
!                     if (fcinfo.argnull[i])
!                     {
!                         *returnDesc = NULL;
!                         return NULL;
!                     }
!                 }
!             }
          }

!         /*
!          * Switch to short-lived context for calling the function or expression.
!          */
!         MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
!
!         /*
!          * Loop to handle the ValuePerCall protocol (which is also the same
!          * behavior needed in the generic ExecEvalExpr path).
!          */
!         for (;;)
          {
+             Datum        result;
+             HeapTuple    tuple;
+
              /*
!              * reset per-tuple memory context before each call of the
!              * function or expression. This cleans up any local memory the
!              * function may leak when called.
               */
!             ResetExprContext(econtext);

              /*
!              * If first time through, build tuplestore for result
               */
              if (first_time)
              {
                  oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
!                 tupstore = tuplestore_begin_heap(true,    /* randomAccess */
!                                                  SortMem);
!                 MemoryContextSwitchTo(oldcontext);
!                 rsinfo.setResult = tupstore;
!             }
!
!             /* Call the function or expression one time */
!             if (direct_function_call)
!             {
!                 fcinfo.isnull = false;
!                 rsinfo.isDone = ExprSingleResult;
!                 result = FunctionCallInvoke(&fcinfo);
!             }
!             else
!             {
!                 result = ExecEvalExpr(funcexpr, econtext,
!                                       &fcinfo.isnull, &rsinfo.isDone);
!             }
!
!             /* Which protocol does function want to use? */
!             if (rsinfo.returnMode == SFRM_ValuePerCall)
!             {
!                 /*
!                  * Check for end of result set.
!                  *
!                  * Note: if function returns an empty set, we don't build a
!                  * tupdesc or tuplestore (since we can't get a tupdesc in the
!                  * function-returning-tuple case)
!                  */
!                 if (rsinfo.isDone == ExprEndResult)
!                     break;
!
!                 /*
!                  * If first time through, build tupdesc for result
!                  */
!                 if (first_time)
!                 {
!                     oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
!                     if (funcrettype == RECORDOID ||
!                         get_typtype(funcrettype) == 'c')
!                     {
!                         /*
!                          * Composite type, so function should have returned a
!                          * TupleTableSlot; use its descriptor
!                          */
!                         slot = (TupleTableSlot *) DatumGetPointer(result);
!                         if (fcinfo.isnull ||
!                             !slot ||
!                             !IsA(slot, TupleTableSlot) ||
!                             !slot->ttc_tupleDescriptor)
!                             elog(ERROR, "ExecMakeTableFunctionResult: Invalid result from function returning tuple");
!                         tupdesc = CreateTupleDescCopy(slot->ttc_tupleDescriptor);
!                         returnsTuple = true;
!                     }
!                     else
!                     {
!                         /*
!                          * Scalar type, so make a single-column descriptor
!                          */
!                         tupdesc = CreateTemplateTupleDesc(1, false);
!                         TupleDescInitEntry(tupdesc,
!                                            (AttrNumber) 1,
!                                            "column",
!                                            funcrettype,
!                                            -1,
!                                            0,
!                                            false);
!                     }
!                     MemoryContextSwitchTo(oldcontext);
!                     rsinfo.setDesc = tupdesc;
!                     first_time = false;
!                 }
!
!                 /*
!                  * Store current resultset item.
!                  */
!                 if (returnsTuple)
                  {
                      slot = (TupleTableSlot *) DatumGetPointer(result);
                      if (fcinfo.isnull ||
                          !slot ||
                          !IsA(slot, TupleTableSlot) ||
!                         TupIsNull(slot))
                          elog(ERROR, "ExecMakeTableFunctionResult: Invalid result from function returning tuple");
!                     tuple = slot->val;
                  }
                  else
                  {
!                     char        nullflag;
!
!                     nullflag = fcinfo.isnull ? 'n' : ' ';
!                     tuple = heap_formtuple(tupdesc, &result, &nullflag);
                  }
!
!                 oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
!                 tuplestore_puttuple(tupstore, tuple);
                  MemoryContextSwitchTo(oldcontext);

!                 /*
!                  * Are we done?
!                  */
!                 if (rsinfo.isDone != ExprMultipleResult)
!                     break;
              }
!             else if (rsinfo.returnMode == SFRM_Materialize)
              {
!                 first_time = false;

!                 /* Done evaluating the set result */
                  break;
+             }
+             else
+                 elog(ERROR, "ExecMakeTableFunctionResult: unknown returnMode %d",
+                      (int) rsinfo.returnMode);
          }

!         if (direct_function_call == false || argDone == ExprSingleResult)
!             break;
      }

      /* If we have a locally-created tupstore, close it up */
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.75
diff -c -r1.75 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c    15 Dec 2002 16:17:58 -0000    1.75
--- src/pl/plpgsql/src/pl_exec.c    16 Dec 2002 21:21:47 -0000
***************
*** 351,357 ****
              MemoryContext oldcxt;

              oldcxt = MemoryContextSwitchTo(estate.tuple_store_cxt);
-             tuplestore_donestoring(estate.tuple_store);
              rsi->setResult = estate.tuple_store;
              if (estate.rettupdesc)
                  rsi->setDesc = CreateTupleDescCopy(estate.rettupdesc);
--- 351,356 ----
***************
*** 1730,1736 ****
  exec_init_tuple_store(PLpgSQL_execstate * estate)
  {
      ReturnSetInfo *rsi = estate->rsi;
-     MemoryContext oldcxt;

      /*
       * Check caller can handle a set result in the way we want
--- 1729,1734 ----
***************
*** 1741,1751 ****
          elog(ERROR, "Set-valued function called in context that cannot accept a set");

      estate->tuple_store_cxt = rsi->econtext->ecxt_per_query_memory;
!
!     oldcxt = MemoryContextSwitchTo(estate->tuple_store_cxt);
!     estate->tuple_store = tuplestore_begin_heap(true, SortMem);
!     MemoryContextSwitchTo(oldcxt);
!
      estate->rettupdesc = rsi->expectedDesc;
  }

--- 1739,1745 ----
          elog(ERROR, "Set-valued function called in context that cannot accept a set");

      estate->tuple_store_cxt = rsi->econtext->ecxt_per_query_memory;
!     estate->tuple_store = rsi->setResult;
      estate->rettupdesc = rsi->expectedDesc;
  }