mirror/postgresql
2
0
Fork 0
mirror of https://git.postgresql.org/git/postgresql.git synced 2025-01-12 18:34:36 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix bugs in plpgsql's handling of CALL argument lists.

Browse Source 
exec_stmt_call() tried to extract information out of a CALL statement's
argument list without using expand_function_arguments(), apparently in
the hope of saving a few nanoseconds by not processing defaulted
arguments.  It got that quite wrong though, leading to crashes with
named arguments, as well as failure to enforce writability of the
argument for a defaulted INOUT parameter.  Fix and simplify the logic
by using expand_function_arguments() before examining the list.

Also, move the argument-examination to just after producing the CALL
command's plan, before invoking the called procedure.  This ensures
that we'll track possible changes in the procedure's argument list
correctly, and avoids a hazard of the plan cache being flushed while
the procedure executes.

Also fix assorted falsehoods and omissions in associated documentation.

Per bug #15477 from Alexey Stepanov.

Patch by me, with some help from Pavel Stehule.  Back-patch to v11.

Discussion: https://postgr.es/m/15477-86075b1d1d319e0a@postgresql.org
Discussion: https://postgr.es/m/CAFj8pRA6UsujpTs9Sdwmk-R6yQykPx46wgjj+YZ7zxm4onrDyw@mail.gmail.com
This commit is contained in:
Tom Lane 2018-11-04 13:25:39 -05:00
parent 3e0b05a756
commit 15c7293477
5 changed files with 300 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
doc/src/sgml/plpgsql.sgml
View File

@ -1864,15 +1864,29 @@ SELECT * FROM get_available_flightid(CURRENT_DATE);
<para>
A procedure does not have a return value. A procedure can therefore end
without a <command>RETURN</command> statement. If
a <command>RETURN</command> statement is desired to exit the code early,
then <symbol>NULL</symbol> must be returned. Returning any other value
will result in an error.
without a <command>RETURN</command> statement. If you wish to use
a <command>RETURN</command> statement to exit the code early, write
just <command>RETURN</command> with no expression.
</para>
<para>
If a procedure has output parameters, then the output values can be
assigned to the parameters as if they were variables. For example:
If the procedure has output parameters, the final values of the output
parameter variables will be returned to the caller.
</para>
</sect2>
<sect2 id="plpgsql-statements-calling-procedure">
<title>Calling a Procedure</title>
<para>
A <application>PL/pgSQL</application> function, procedure,
or <command>DO</command> block can call a procedure
using <command>CALL</command>. Output parameters are handled
differently from the way that <command>CALL</command> works in plain
SQL. Each <literal>INOUT</literal> parameter of the procedure must
correspond to a variable in the <command>CALL</command> statement, and
whatever the procedure returns is assigned back to that variable after
it returns. For example:
<programlisting>
CREATE PROCEDURE triple(INOUT x int)
LANGUAGE plpgsql
@ -1882,7 +1896,13 @@ BEGIN
END;
$$;
CALL triple(5);
DO $$
DECLARE myvar int := 5;
BEGIN
CALL triple(myvar);
RAISE NOTICE 'myvar = %', myvar; -- prints 15
END
$$;
</programlisting>
</para>
</sect2>

11
doc/src/sgml/ref/call.sgml
View File

@ -33,7 +33,8 @@ CALL <replaceable class="parameter">name</replaceable> ( [ <replaceable class="p
</para>
<para>
If the procedure has output arguments, then a result row will be returned.
If the procedure has any output parameters, then a result row will be
returned, containing the values of those parameters.
</para>
</refsect1>
@ -54,7 +55,7 @@ CALL <replaceable class="parameter">name</replaceable> ( [ <replaceable class="p
<term><replaceable class="parameter">argument</replaceable></term>
<listitem>
<para>
An argument for the procedure call.
An input argument for the procedure call.
See <xref linkend="sql-syntax-calling-funcs"/> for the full details on
function and procedure call syntax, including use of named parameters.
</para>
@ -81,6 +82,12 @@ CALL <replaceable class="parameter">name</replaceable> ( [ <replaceable class="p
Transaction control statements are only allowed if <command>CALL</command>
is executed in its own transaction.
</para>
<para>
<application>PL/pgSQL</application> handles output parameters
in <command>CALL</command> commands differently;
see <xref linkend="plpgsql-statements-calling-procedure"/>.
</para>
</refsect1>
<refsect1>

84
src/pl/plpgsql/src/expected/plpgsql_call.out
View File

@ -114,7 +114,7 @@ BEGIN
RAISE INFO 'x = %, y = %', x, y;
END;
$$;
ERROR: argument 2 is an output argument but is not writable
ERROR: procedure parameter "b" is an output parameter but corresponding argument is not writable
CONTEXT: PL/pgSQL function inline_code_block line 6 at CALL
DO
LANGUAGE plpgsql
@ -227,28 +227,43 @@ $$;
DO $$
DECLARE _a int; _b int; _c int;
BEGIN
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, _b);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, b => _b);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, _b, _c);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, c => _c, b => _b);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(c => _c, b => _b, a => _a);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
END
$$;
NOTICE: a: 10, b: 30, c: 11
NOTICE: _a: 100, _b: 40, _c: 50
NOTICE: a: 10, b: 30, c: 11
NOTICE: _a: 100, _b: 40, _c: 50
NOTICE: a: 10, b: 30, c: 50
NOTICE: _a: 100, _b: 40, _c: -500
NOTICE: a: 10, b: 30, c: 50
NOTICE: _a: 100, _b: 40, _c: -500
NOTICE: a: 10, b: 30, c: 50
NOTICE: _a: 100, _b: 40, _c: -500
DO $$
DECLARE _a int; _b int; _c int;
BEGIN
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, _b); -- fail, no output argument for c
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
END
$$;
ERROR: procedure parameter "c" is an output parameter but corresponding argument is not writable
CONTEXT: PL/pgSQL function inline_code_block line 5 at CALL
DO $$
DECLARE _a int; _b int; _c int;
BEGIN
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, b => _b); -- fail, no output argument for c
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
END
$$;
ERROR: procedure parameter "c" is an output parameter but corresponding argument is not writable
CONTEXT: PL/pgSQL function inline_code_block line 5 at CALL
-- transition variable assignment
TRUNCATE test1;
CREATE FUNCTION triggerfunc1() RETURNS trigger
@ -276,3 +291,50 @@ DROP PROCEDURE test_proc1;
DROP PROCEDURE test_proc3;
DROP PROCEDURE test_proc4;
DROP TABLE test1;
-- more checks for named-parameter handling
CREATE PROCEDURE p1(v_cnt int, v_Text inout text = NULL)
AS $$
BEGIN
v_Text := 'v_cnt = ' || v_cnt;
END
$$ LANGUAGE plpgsql;
DO $$
DECLARE
v_Text text;
v_cnt integer := 42;
BEGIN
CALL p1(v_cnt := v_cnt); -- error, must supply something for v_Text
RAISE NOTICE '%', v_Text;
END;
$$;
ERROR: procedure parameter "v_text" is an output parameter but corresponding argument is not writable
CONTEXT: PL/pgSQL function inline_code_block line 6 at CALL
DO $$
DECLARE
v_Text text;
v_cnt integer := 42;
BEGIN
CALL p1(v_cnt := v_cnt, v_Text := v_Text);
RAISE NOTICE '%', v_Text;
END;
$$;
NOTICE: v_cnt = 42
DO $$
DECLARE
v_Text text;
BEGIN
CALL p1(10, v_Text := v_Text);
RAISE NOTICE '%', v_Text;
END;
$$;
NOTICE: v_cnt = 10
DO $$
DECLARE
v_Text text;
v_cnt integer;
BEGIN
CALL p1(v_Text := v_Text, v_cnt := v_cnt);
RAISE NOTICE '%', v_Text;
END;
$$;
NOTICE: <NULL>

221
src/pl/plpgsql/src/pl_exec.c
View File

@ -30,6 +30,7 @@
#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"
@ -2071,7 +2072,6 @@ static int
exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
{
PLpgSQL_expr *expr = stmt->expr;
SPIPlanPtr plan;
ParamListInfo paramLI;
LocalTransactionId before_lxid;
LocalTransactionId after_lxid;
@ -2080,6 +2080,8 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
if (expr->plan == NULL)
{
SPIPlanPtr plan;
/*
* Don't save the plan if not in atomic context. Otherwise,
* transaction ends would cause errors about plancache leaks. XXX
@ -2093,7 +2095,117 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
* snapshot management in SPI_execute*, so don't let it do it.
* Instead, we set the snapshots ourselves below.
*/
expr->plan->no_snapshots = true;
plan = expr->plan;
plan->no_snapshots = true;
/*
* We 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. (We do this each time the
* plan changes, in case the procedure's argument list has changed.)
*/
if (stmt->is_call)
{
Node *node;
FuncExpr *funcexpr;
HeapTuple func_tuple;
List *funcargs;
Oid *argtypes;
char **argnames;
char *argmodes;
MemoryContext oldcontext;
PLpgSQL_row *row;
int nfields;
int i;
ListCell *lc;
/*
* 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 names and 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
{
/* report error using parameter name, if available */
if (argnames && argnames[i] && argnames[i][0])
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("procedure parameter \"%s\" is an output parameter but corresponding argument is not writable",
argnames[i])));
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("procedure parameter %d is an output parameter but corresponding argument is not writable",
i + 1)));
}
}
i++;
}
row->nfields = nfields;
stmt->target = (PLpgSQL_variable *) row;
}
}
paramLI = setup_param_list(estate, expr);
@ -2127,17 +2239,15 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
}
PG_END_TRY();
plan = expr->plan;
if (expr->plan && !expr->plan->saved)
expr->plan = NULL;
after_lxid = MyProc->lxid;
if (rc < 0)
elog(ERROR, "SPI_execute_plan_with_paramlist failed executing query \"%s\": %s",
expr->query, SPI_result_code_string(rc));
after_lxid = MyProc->lxid;
if (before_lxid == after_lxid)
{
/*
@ -2158,105 +2268,16 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
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;
/*
* 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;
}
elog(ERROR, "DO statement returned a row");
exec_move_row(estate, stmt->target, tuptab->vals[0], tuptab->tupdesc);
}

76
src/pl/plpgsql/src/sql/plpgsql_call.sql
View File

@ -206,21 +206,36 @@ $$;
DO $$
DECLARE _a int; _b int; _c int;
BEGIN
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, _b);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, b => _b);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, _b, _c);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, c => _c, b => _b);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(c => _c, b => _b, a => _a);
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
END
$$;
DO $$
DECLARE _a int; _b int; _c int;
BEGIN
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, _b); -- fail, no output argument for c
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
END
$$;
DO $$
DECLARE _a int; _b int; _c int;
BEGIN
_a := 10; _b := 30; _c := 50;
CALL test_proc8c(_a, b => _b); -- fail, no output argument for c
RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
END
$$;
-- transition variable assignment
@ -251,3 +266,52 @@ DROP PROCEDURE test_proc3;
DROP PROCEDURE test_proc4;
DROP TABLE test1;
-- more checks for named-parameter handling
CREATE PROCEDURE p1(v_cnt int, v_Text inout text = NULL)
AS $$
BEGIN
v_Text := 'v_cnt = ' || v_cnt;
END
$$ LANGUAGE plpgsql;
DO $$
DECLARE
v_Text text;
v_cnt integer := 42;
BEGIN
CALL p1(v_cnt := v_cnt); -- error, must supply something for v_Text
RAISE NOTICE '%', v_Text;
END;
$$;
DO $$
DECLARE
v_Text text;
v_cnt integer := 42;
BEGIN
CALL p1(v_cnt := v_cnt, v_Text := v_Text);
RAISE NOTICE '%', v_Text;
END;
$$;
DO $$
DECLARE
v_Text text;
BEGIN
CALL p1(10, v_Text := v_Text);
RAISE NOTICE '%', v_Text;
END;
$$;
DO $$
DECLARE
v_Text text;
v_cnt integer;
BEGIN
CALL p1(v_Text := v_Text, v_cnt := v_cnt);
RAISE NOTICE '%', v_Text;
END;
$$;