From: Tom Lane <tgl@sss.pgh.pa.us>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean"
Date: Sun, 04 Nov 2018 11:14:52 -0500
Message-ID: <10615.1541348092@sss.pgh.pa.us> (raw)
In-Reply-To: <CAFj8pRAcdrRogAC2DU9in=gT1h3y1AgpJnAnDmzho46NS7fJ-w@mail.gmail.com>
References: <CAFj8pRA6UsujpTs9Sdwmk-R6yQykPx46wgjj+YZ7zxm4onrDyw@mail.gmail.com>
<CAFj8pRBdNqq8+SsChS8F5s9ttkdBSTPu=cq-fLx51FF7Tk_LkA@mail.gmail.com>
<CAFj8pRCF3pia7gScM0jh7vY9rEuqN7fbnUV+jymRTLEXZeRDNA@mail.gmail.com>
<CAFj8pRD2=MGU8ZshvmCjv5KG+3EngRm0+8pFLsjhG5bUhUGU2Q@mail.gmail.com>
<CAFj8pRCehpRXn2vSq8Q+dQL2sQ0XXt0rOukFb0tZ5F7RFdH+DA@mail.gmail.com>
<1d846492-3822-616b-7246-91b429cc5dbb@2ndquadrant.com>
<CAFj8pRB3ZPu_SUDamXGA+wXw2aMmn0oLmgvsdMT1oAyJUMgFZA@mail.gmail.com>
<26116.1541278051@sss.pgh.pa.us>
<28551.1541281624@sss.pgh.pa.us>
<CAFj8pRDUL=nC62L-AKKFDEo1en1ZT1otLfPdXkCjm8MHxif8PA@mail.gmail.com>
<7102.1541346845@sss.pgh.pa.us>
<CAFj8pRAcdrRogAC2DU9in=gT1h3y1AgpJnAnDmzho46NS7fJ-w@mail.gmail.com>
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
Attachments:call-param-matching-fix-0.1.patch (text/x-diff, 6.2K) downloaddiff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.cindex 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;
}
=
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pavel.stehule@gmail.com, peter.eisentraut@2ndquadrant.com
Subject: Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean"
In-Reply-To: <10615.1541348092@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox