Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean"

Pavel Stehule <pavel.stehule@gmail.com> writes:
> ne 4. 11. 2018 v 16:54 odesilatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
>> In short, I think it's a bug that we allow the above.  If you
>> want to keep the must-be-a-variable error then it should apply in
>> this case too.

> I agree. This should be prohibited from PLpgSQL.

OK.  In that case I'll run with my patch.  The attached is what I had
code-wise, but I've not changed the regression tests to match ... gotta
go fix the docs too I guess.

            regards, tom lane

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 4552638..4e6d5b1 100644
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 30,35 ****
--- 30,36 ----
  #include "funcapi.h"
  #include "miscadmin.h"
  #include "nodes/nodeFuncs.h"
+ #include "optimizer/clauses.h"
  #include "optimizer/planner.h"
  #include "parser/parse_coerce.h"
  #include "parser/scansup.h"
*************** exec_stmt_call(PLpgSQL_execstate *estate
*** 2158,2260 ****
          plpgsql_create_econtext(estate);
      }

      if (SPI_processed == 1)
      {
          SPITupleTable *tuptab = SPI_tuptable;

          /*
!          * Construct a dummy target row based on the output arguments of the
!          * procedure call.
           */
          if (!stmt->target)
          {
              Node       *node;
-             ListCell   *lc;
              FuncExpr   *funcexpr;
!             int            i;
!             HeapTuple    tuple;
              Oid           *argtypes;
              char      **argnames;
              char       *argmodes;
              MemoryContext oldcontext;
              PLpgSQL_row *row;
              int            nfields;

              /*
!              * Get the original CallStmt
               */
!             node = linitial_node(Query, ((CachedPlanSource *)
linitial(plan->plancache_list))->query_list)->utilityStmt;
!             if (!IsA(node, CallStmt))
!                 elog(ERROR, "returned row from not a CallStmt");

!             funcexpr = castNode(CallStmt, node)->funcexpr;

              /*
!              * Get the argument modes
               */
!             tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcexpr->funcid));
!             if (!HeapTupleIsValid(tuple))
!                 elog(ERROR, "cache lookup failed for function %u", funcexpr->funcid);
!             get_func_arg_info(tuple, &argtypes, &argnames, &argmodes);
!             ReleaseSysCache(tuple);

              /*
!              * Construct row
               */
              oldcontext = MemoryContextSwitchTo(estate->func->fn_cxt);

!             row = palloc0(sizeof(*row));
              row->dtype = PLPGSQL_DTYPE_ROW;
              row->refname = "(unnamed row)";
              row->lineno = -1;
!             row->varnos = palloc(sizeof(int) * FUNC_MAX_ARGS);

              nfields = 0;
              i = 0;
!             foreach(lc, funcexpr->args)
              {
                  Node       *n = lfirst(lc);

!                 if (argmodes && argmodes[i] == PROARGMODE_INOUT)
                  {
                      if (IsA(n, Param))
                      {
!                         Param       *param = castNode(Param, n);

                          /* paramid is offset by 1 (see make_datum_param()) */
                          row->varnos[nfields++] = param->paramid - 1;
                      }
-                     else if (IsA(n, NamedArgExpr))
-                     {
-                         NamedArgExpr *nexpr = castNode(NamedArgExpr, n);
-                         Param       *param;
-
-                         if (!IsA(nexpr->arg, Param))
-                             ereport(ERROR,
-                                     (errcode(ERRCODE_SYNTAX_ERROR),
-                                      errmsg("argument %d is an output argument but is not writable", i + 1)));
-
-                         param = castNode(Param, nexpr->arg);
-
-                         /*
-                          * Named arguments must be after positional arguments,
-                          * so we can increase nfields.
-                          */
-                         row->varnos[nexpr->argnumber] = param->paramid - 1;
-                         nfields++;
-                     }
                      else
                          ereport(ERROR,
                                  (errcode(ERRCODE_SYNTAX_ERROR),
!                                  errmsg("argument %d is an output argument but is not writable", i + 1)));
                  }
                  i++;
              }

              row->nfields = nfields;

-             MemoryContextSwitchTo(oldcontext);
-
              stmt->target = (PLpgSQL_variable *) row;
          }

--- 2159,2268 ----
          plpgsql_create_econtext(estate);
      }

+     /*
+      * Check result rowcount; if there's one row, assign procedure's output
+      * values back to the appropriate variables.
+      */
      if (SPI_processed == 1)
      {
          SPITupleTable *tuptab = SPI_tuptable;

          /*
!          * If first time through, construct a DTYPE_ROW datum representing the
!          * plpgsql variables associated with the procedure's output arguments.
!          * Then we can use exec_move_row() to do the assignments.
           */
          if (!stmt->target)
          {
              Node       *node;
              FuncExpr   *funcexpr;
!             HeapTuple    func_tuple;
!             List       *funcargs;
              Oid           *argtypes;
              char      **argnames;
              char       *argmodes;
              MemoryContext oldcontext;
              PLpgSQL_row *row;
              int            nfields;
+             ListCell   *lc;
+             int            i;

              /*
!              * Get the parsed CallStmt, and look up the called procedure
               */
!             node = linitial_node(Query,
!                                  ((CachedPlanSource *) linitial(plan->plancache_list))->query_list)->utilityStmt;
!             if (node == NULL || !IsA(node, CallStmt))
!                 elog(ERROR, "query for CALL statement is not a CallStmt");

!             funcexpr = ((CallStmt *) node)->funcexpr;
!
!             func_tuple = SearchSysCache1(PROCOID,
!                                          ObjectIdGetDatum(funcexpr->funcid));
!             if (!HeapTupleIsValid(func_tuple))
!                 elog(ERROR, "cache lookup failed for function %u",
!                      funcexpr->funcid);

              /*
!              * Extract function arguments, and expand any named-arg notation
               */
!             funcargs = expand_function_arguments(funcexpr->args,
!                                                  funcexpr->funcresulttype,
!                                                  func_tuple);

              /*
!              * Get the argument modes, too
!              */
!             get_func_arg_info(func_tuple, &argtypes, &argnames, &argmodes);
!
!             ReleaseSysCache(func_tuple);
!
!             /*
!              * Begin constructing row Datum
               */
              oldcontext = MemoryContextSwitchTo(estate->func->fn_cxt);

!             row = (PLpgSQL_row *) palloc0(sizeof(PLpgSQL_row));
              row->dtype = PLPGSQL_DTYPE_ROW;
              row->refname = "(unnamed row)";
              row->lineno = -1;
!             row->varnos = (int *) palloc(sizeof(int) * list_length(funcargs));

+             MemoryContextSwitchTo(oldcontext);
+
+             /*
+              * Examine procedure's argument list.  Each output arg position
+              * should be an unadorned plpgsql variable (datum), which we can
+              * insert into the row Datum.
+              */
              nfields = 0;
              i = 0;
!             foreach(lc, funcargs)
              {
                  Node       *n = lfirst(lc);

!                 if (argmodes &&
!                     (argmodes[i] == PROARGMODE_INOUT ||
!                      argmodes[i] == PROARGMODE_OUT))
                  {
                      if (IsA(n, Param))
                      {
!                         Param       *param = (Param *) n;

                          /* paramid is offset by 1 (see make_datum_param()) */
                          row->varnos[nfields++] = param->paramid - 1;
                      }
                      else
                          ereport(ERROR,
                                  (errcode(ERRCODE_SYNTAX_ERROR),
!                                  errmsg("CALL argument %d is an output argument but is not writable",
!                                         i + 1)));
                  }
                  i++;
              }

              row->nfields = nfields;

              stmt->target = (PLpgSQL_variable *) row;
          }