From 5b9f1506e73826f4f6ff567e54b12c4e232a4263 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 12 Mar 2018 21:39:26 -0400 Subject: [PATCH v4] Support INOUT parameters in procedures In a top-level CALL, the values of INOUT parameters will be returned as a result row. In PL/pgSQL, the values are assigned back to the input parameters. In other languages, the same convention as for return a record from a function is used. That does not require any code changes in the PL implementations. Reviewed-by: Pavel Stehule --- doc/src/sgml/plperl.sgml | 14 +++ doc/src/sgml/plpgsql.sgml | 16 +++ doc/src/sgml/plpython.sgml | 11 ++ doc/src/sgml/pltcl.sgml | 12 ++ doc/src/sgml/ref/create_procedure.sgml | 7 +- src/backend/catalog/pg_proc.c | 4 +- src/backend/commands/functioncmds.c | 51 +++++++-- src/backend/executor/functions.c | 51 +++++++++ src/backend/tcop/utility.c | 3 +- src/backend/utils/fmgr/funcapi.c | 11 +- src/include/commands/defrem.h | 3 +- src/include/executor/functions.h | 2 + src/include/funcapi.h | 3 +- src/pl/plperl/expected/plperl_call.out | 25 +++++ src/pl/plperl/sql/plperl_call.sql | 22 ++++ src/pl/plpgsql/src/expected/plpgsql_call.out | 89 +++++++++++++++ .../plpgsql/src/expected/plpgsql_transaction.out | 2 +- src/pl/plpgsql/src/pl_comp.c | 10 +- src/pl/plpgsql/src/pl_exec.c | 125 ++++++++++++++++++++- src/pl/plpgsql/src/pl_funcs.c | 25 +++++ src/pl/plpgsql/src/pl_gram.y | 38 +++++-- src/pl/plpgsql/src/pl_scanner.c | 1 + src/pl/plpgsql/src/plpgsql.h | 12 ++ src/pl/plpgsql/src/sql/plpgsql_call.sql | 108 ++++++++++++++++++ src/pl/plpython/expected/plpython_call.out | 23 ++++ src/pl/plpython/plpy_exec.c | 24 ++-- src/pl/plpython/sql/plpython_call.sql | 20 ++++ src/pl/tcl/expected/pltcl_call.out | 26 +++++ src/pl/tcl/sql/pltcl_call.sql | 23 ++++ src/test/regress/expected/create_procedure.out | 21 ++++ src/test/regress/sql/create_procedure.sql | 19 ++++ 31 files changed, 752 insertions(+), 49 deletions(-) diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml index cff7a847de..9295c03db9 100644 --- a/doc/src/sgml/plperl.sgml +++ b/doc/src/sgml/plperl.sgml @@ -278,6 +278,20 @@ PL/Perl Functions and Arguments hash will be returned as null values. + + Similarly, output parameters of procedures can be returned as a hash + reference: + + +CREATE PROCEDURE perl_triple(INOUT a integer, INOUT b integer) AS $$ + my ($a, $b) = @_; + return {a => $a * 3, b => $b * 3}; +$$ LANGUAGE plperl; + +CALL perl_triple(5, 10); + + + PL/Perl functions can also return sets of either scalar or composite types. Usually you'll want to return rows one at a diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index c1e3c6a19d..6c25116538 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -1870,6 +1870,22 @@ Returning From a Procedure then NULL must be returned. Returning any other value will result in an error. + + + If a procedure has output parameters, then the output values can be + assigned to the parameters as if they were variables. For example: + +CREATE PROCEDURE triple(INOUT x int) +LANGUAGE plpgsql +AS $$ +BEGIN + x := x * 3; +END; +$$; + +CALL triple(5); + + diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml index ba79beb743..3b7974690e 100644 --- a/doc/src/sgml/plpython.sgml +++ b/doc/src/sgml/plpython.sgml @@ -649,6 +649,17 @@ Composite Types $$ LANGUAGE plpythonu; SELECT * FROM multiout_simple(); + + + + + Output parameters of procedures are passed back the same way. For example: + +CREATE PROCEDURE python_triple(INOUT a integer, INOUT b integer) AS $$ +return (a * 3, b * 3) +$$ LANGUAGE plpythonu; + +CALL python_triple(5, 10); diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml index a834ab8862..121260379a 100644 --- a/doc/src/sgml/pltcl.sgml +++ b/doc/src/sgml/pltcl.sgml @@ -186,6 +186,18 @@ PL/Tcl Functions and Arguments + + Output parameters of procedures are returned in the same way, for example: + + +CREATE PROCEDURE tcl_triple(INOUT a integer, INOUT b integer) AS $$ + return [list a [expr {$1 * 3}] b [expr {$2 * 3}]] +$$ LANGUAGE pltcl; + +CALL tcl_triple(5, 10); + + + The result list can be made from an array representation of the diff --git a/doc/src/sgml/ref/create_procedure.sgml b/doc/src/sgml/ref/create_procedure.sgml index bbf8b03d04..f3c3bb006c 100644 --- a/doc/src/sgml/ref/create_procedure.sgml +++ b/doc/src/sgml/ref/create_procedure.sgml @@ -96,8 +96,11 @@ Parameters - The mode of an argument: IN or VARIADIC. - If omitted, the default is IN. + The mode of an argument: IN, + INOUT, or VARIADIC. If omitted, + the default is IN. (OUT + arguments are currently not supported for procedures. Use + INOUT instead.) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 40e579f95d..466ff038e7 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -438,7 +438,8 @@ ProcedureCreate(const char *procedureName, TupleDesc newdesc; olddesc = build_function_result_tupdesc_t(oldtup); - newdesc = build_function_result_tupdesc_d(allParameterTypes, + newdesc = build_function_result_tupdesc_d(prokind, + allParameterTypes, parameterModes, parameterNames); if (olddesc == NULL && newdesc == NULL) @@ -925,6 +926,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS) querytree_sublist); } + check_sql_fn_statements(querytree_list); (void) check_sql_fn_retval(funcoid, proc->prorettype, querytree_list, NULL, NULL); diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index b1f87d056e..6267c785db 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -68,6 +68,7 @@ #include "utils/memutils.h" #include "utils/rel.h" #include "utils/syscache.h" +#include "utils/typcache.h" #include "utils/tqual.h" /* @@ -281,10 +282,11 @@ interpret_function_parameter_list(ParseState *pstate, if (objtype == OBJECT_PROCEDURE) { - if (fp->mode == FUNC_PARAM_OUT || fp->mode == FUNC_PARAM_INOUT) + if (fp->mode == FUNC_PARAM_OUT) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - (errmsg("procedures cannot have OUT parameters")))); + (errmsg("procedures cannot have OUT parameters"), + errhint("INOUT parameters are permitted.")))); } /* handle input parameters */ @@ -302,7 +304,9 @@ interpret_function_parameter_list(ParseState *pstate, /* handle output parameters */ if (fp->mode != FUNC_PARAM_IN && fp->mode != FUNC_PARAM_VARIADIC) { - if (outCount == 0) /* save first output param's type */ + if (objtype == OBJECT_PROCEDURE) + *requiredResultType = RECORDOID; + else if (outCount == 0) /* save first output param's type */ *requiredResultType = toid; outCount++; } @@ -1003,12 +1007,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt) if (stmt->is_procedure) { - /* - * Sometime in the future, procedures might be allowed to return - * results; for now, they all return VOID. - */ Assert(!stmt->returnType); - prorettype = VOIDOID; + prorettype = requiredResultType ? requiredResultType : VOIDOID; returnsSet = false; } else if (stmt->returnType) @@ -2206,7 +2206,7 @@ ExecuteDoStmt(DoStmt *stmt, bool atomic) * commits that might occur inside the procedure. */ void -ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic) +ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver *dest) { ListCell *lc; FuncExpr *fexpr; @@ -2219,6 +2219,7 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic) EState *estate; ExprContext *econtext; HeapTuple tp; + Datum retval; fexpr = stmt->funcexpr; Assert(fexpr); @@ -2285,7 +2286,37 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic) i++; } - FunctionCallInvoke(&fcinfo); + retval = FunctionCallInvoke(&fcinfo); + + if (fexpr->funcresulttype == RECORDOID && !fcinfo.isnull) + { + HeapTupleHeader td; + Oid tupType; + int32 tupTypmod; + TupleDesc retdesc; + HeapTupleData rettupdata; + TupOutputState *tstate; + TupleTableSlot *slot; + + td = DatumGetHeapTupleHeader(retval); + tupType = HeapTupleHeaderGetTypeId(td); + tupTypmod = HeapTupleHeaderGetTypMod(td); + retdesc = lookup_rowtype_tupdesc(tupType, tupTypmod); + + rettupdata.t_len = HeapTupleHeaderGetDatumLength(td); + ItemPointerSetInvalid(&(rettupdata.t_self)); + rettupdata.t_tableOid = InvalidOid; + rettupdata.t_data = td; + + tstate = begin_tup_output_tupdesc(dest, retdesc); + + slot = ExecStoreTuple(&rettupdata, tstate->slot, InvalidBuffer, false); + tstate->dest->receiveSlot(slot, tstate->dest); + + end_tup_output(tstate); + + ReleaseTupleDesc(retdesc); + } FreeExecutorState(estate); } diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 78bc4ab34b..f336190607 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -721,6 +721,8 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK) list_copy(queryTree_sublist)); } + check_sql_fn_statements(flat_query_list); + /* * Check that the function returns the type it claims to. Although in * simple cases this was already done when the function was defined, we @@ -1486,6 +1488,55 @@ ShutdownSQLFunction(Datum arg) fcache->shutdown_reg = false; } +/* + * check_sql_fn_statements + * + * Check statements in an SQL function. Error out if there is anything that + * is not acceptable. + */ +void +check_sql_fn_statements(List *queryTreeList) +{ + ListCell *lc; + + foreach(lc, queryTreeList) + { + Query *query = lfirst_node(Query, lc); + + /* + * Disallow procedures with output parameters. The current + * implementation would just throw the output values away, unless the + * statement is the last one. Per SQL standard, we should assign the + * output values by name. By disallowing this here, we preserve an + * opportunity for future improvement. + */ + if (query->commandType == CMD_UTILITY && + IsA(query->utilityStmt, CallStmt)) + { + CallStmt *stmt = castNode(CallStmt, query->utilityStmt); + HeapTuple tuple; + int numargs; + Oid *argtypes; + char **argnames; + char *argmodes; + int i; + + tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(stmt->funcexpr->funcid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for function %u", stmt->funcexpr->funcid); + numargs = get_func_arg_info(tuple, &argtypes, &argnames, &argmodes); + ReleaseSysCache(tuple); + + for (i = 0; i < numargs; i++) + { + if (argmodes && (argmodes[i] == PROARGMODE_INOUT || argmodes[i] == PROARGMODE_OUT)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("calling procedures with output parameters is not supported in SQL functions"))); + } + } + } +} /* * check_sql_fn_retval() -- check return value of a list of sql parse trees. diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index f78efdf359..6effe031f8 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -661,7 +661,8 @@ standard_ProcessUtility(PlannedStmt *pstmt, case T_CallStmt: ExecuteCallStmt(castNode(CallStmt, parsetree), params, - (context != PROCESS_UTILITY_TOPLEVEL || IsTransactionBlock())); + (context != PROCESS_UTILITY_TOPLEVEL || IsTransactionBlock()), + dest); break; case T_ClusterStmt: diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c index c0076bfce3..20f60392af 100644 --- a/src/backend/utils/fmgr/funcapi.c +++ b/src/backend/utils/fmgr/funcapi.c @@ -1205,7 +1205,8 @@ build_function_result_tupdesc_t(HeapTuple procTuple) if (isnull) proargnames = PointerGetDatum(NULL); /* just to be sure */ - return build_function_result_tupdesc_d(proallargtypes, + return build_function_result_tupdesc_d(procform->prokind, + proallargtypes, proargmodes, proargnames); } @@ -1218,10 +1219,12 @@ build_function_result_tupdesc_t(HeapTuple procTuple) * convenience of ProcedureCreate, which needs to be able to compute the * tupledesc before actually creating the function. * - * Returns NULL if there are not at least two OUT or INOUT arguments. + * For functions (but not for procedures), returns NULL if there are not at + * least two OUT or INOUT arguments. */ TupleDesc -build_function_result_tupdesc_d(Datum proallargtypes, +build_function_result_tupdesc_d(char prokind, + Datum proallargtypes, Datum proargmodes, Datum proargnames) { @@ -1311,7 +1314,7 @@ build_function_result_tupdesc_d(Datum proallargtypes, * If there is no output argument, or only one, the function does not * return tuples. */ - if (numoutargs < 2) + if (numoutargs < 2 && prokind != PROKIND_PROCEDURE) return NULL; desc = CreateTemplateTupleDesc(numoutargs, false); diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index c829abfea7..8fc9e424cf 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -17,6 +17,7 @@ #include "catalog/objectaddress.h" #include "nodes/params.h" #include "nodes/parsenodes.h" +#include "tcop/dest.h" #include "utils/array.h" /* commands/dropcmds.c */ @@ -62,7 +63,7 @@ extern void DropTransformById(Oid transformOid); extern void IsThereFunctionInNamespace(const char *proname, int pronargs, oidvector *proargtypes, Oid nspOid); extern void ExecuteDoStmt(DoStmt *stmt, bool atomic); -extern void ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic); +extern void ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver *dest); extern Oid get_cast_oid(Oid sourcetypeid, Oid targettypeid, bool missing_ok); extern Oid get_transform_oid(Oid type_id, Oid lang_id, bool missing_ok); extern void interpret_function_parameter_list(ParseState *pstate, diff --git a/src/include/executor/functions.h b/src/include/executor/functions.h index e7454ee790..a309809ba8 100644 --- a/src/include/executor/functions.h +++ b/src/include/executor/functions.h @@ -29,6 +29,8 @@ extern SQLFunctionParseInfoPtr prepare_sql_fn_parse_info(HeapTuple procedureTupl extern void sql_fn_parser_setup(struct ParseState *pstate, SQLFunctionParseInfoPtr pinfo); +extern void check_sql_fn_statements(List *queryTreeList); + extern bool check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList, bool *modifyTargetList, diff --git a/src/include/funcapi.h b/src/include/funcapi.h index c2da2eb157..01aa208c5e 100644 --- a/src/include/funcapi.h +++ b/src/include/funcapi.h @@ -187,7 +187,8 @@ extern int get_func_input_arg_names(Datum proargnames, Datum proargmodes, extern int get_func_trftypes(HeapTuple procTup, Oid **p_trftypes); extern char *get_func_result_name(Oid functionId); -extern TupleDesc build_function_result_tupdesc_d(Datum proallargtypes, +extern TupleDesc build_function_result_tupdesc_d(char prokind, + Datum proallargtypes, Datum proargmodes, Datum proargnames); extern TupleDesc build_function_result_tupdesc_t(HeapTuple procTuple); diff --git a/src/pl/plperl/expected/plperl_call.out b/src/pl/plperl/expected/plperl_call.out index 4bccfcb7c8..a9dd3e74b4 100644 --- a/src/pl/plperl/expected/plperl_call.out +++ b/src/pl/plperl/expected/plperl_call.out @@ -23,6 +23,31 @@ SELECT * FROM test1; 55 (1 row) +-- OUT parameters +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE plperl +AS $$ +my ($a) = @_; +return { a => "$a+$a" }; +$$; +CALL test_proc5('abc'); + a +--------- + abc+abc +(1 row) + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE plperl +AS $$ +my ($a, $b, $c) = @_; +return { b => $b * $a, c => $c * $a }; +$$; +CALL test_proc6(2, 3, 4); + b | c +---+--- + 6 | 8 +(1 row) + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; DROP PROCEDURE test_proc3; diff --git a/src/pl/plperl/sql/plperl_call.sql b/src/pl/plperl/sql/plperl_call.sql index bd2b63b418..64a09e1f31 100644 --- a/src/pl/plperl/sql/plperl_call.sql +++ b/src/pl/plperl/sql/plperl_call.sql @@ -29,6 +29,28 @@ CREATE PROCEDURE test_proc3(x int) SELECT * FROM test1; +-- OUT parameters + +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE plperl +AS $$ +my ($a) = @_; +return { a => "$a+$a" }; +$$; + +CALL test_proc5('abc'); + + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE plperl +AS $$ +my ($a, $b, $c) = @_; +return { b => $b * $a, c => $c * $a }; +$$; + +CALL test_proc6(2, 3, 4); + + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; DROP PROCEDURE test_proc3; diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out index 2f3adcd8d8..1d042ae52f 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_call.out +++ b/src/pl/plpgsql/src/expected/plpgsql_call.out @@ -53,6 +53,95 @@ SELECT * FROM test1; 66 (2 rows) +-- OUT parameters +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE plpgsql +AS $$ +BEGIN + a := a || '+' || a; +END; +$$; +CALL test_proc5('abc'); + a +--------- + abc+abc +(1 row) + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE plpgsql +AS $$ +BEGIN + b := b * a; + c := c * a; +END; +$$; +CALL test_proc6(2, 3, 4); + b | c +---+--- + 6 | 8 +(1 row) + +DO +LANGUAGE plpgsql +$$ +DECLARE + x int := 3; + y int := 4; +BEGIN + CALL test_proc6(2, x, y); + RAISE INFO 'x = %, y = %', x, y; +END; +$$; +INFO: x = 6, y = 8 +DO +LANGUAGE plpgsql +$$ +DECLARE + x int := 3; + y int := 4; +BEGIN + CALL test_proc6(2, x + 1, y); -- error + RAISE INFO 'x = %, y = %', x, y; +END; +$$; +ERROR: argument 2 is an output parameter but is not writable +CONTEXT: PL/pgSQL function inline_code_block line 6 at CALL +DO +LANGUAGE plpgsql +$$ +DECLARE + x int := 3; + y int := 4; +BEGIN + FOR i IN 1..5 LOOP + CALL test_proc6(i, x, y); + RAISE INFO 'x = %, y = %', x, y; + END LOOP; +END; +$$; +INFO: x = 3, y = 4 +INFO: x = 6, y = 8 +INFO: x = 18, y = 24 +INFO: x = 72, y = 96 +INFO: x = 360, y = 480 +-- recursive with output parameters +CREATE PROCEDURE test_proc7(x int, INOUT a int, INOUT b numeric) +LANGUAGE plpgsql +AS $$ +BEGIN +IF x > 1 THEN + a := x / 10; + b := x / 2; + CALL test_proc7(b::int, a, b); +END IF; +END; +$$; +CALL test_proc7(100, -1, -1); + a | b +---+--- + 0 | 1 +(1 row) + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc3; DROP PROCEDURE test_proc4; diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out index 8ec22c646c..ce66487137 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out +++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out @@ -98,7 +98,7 @@ SELECT transaction_test3(); ERROR: invalid transaction termination CONTEXT: PL/pgSQL function transaction_test1() line 6 at COMMIT SQL statement "CALL transaction_test1()" -PL/pgSQL function transaction_test3() line 3 at SQL statement +PL/pgSQL function transaction_test3() line 3 at CALL SELECT * FROM test1; a | b ---+--- diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 391ec41b80..b1a0c1cc4f 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -475,11 +475,11 @@ do_compile(FunctionCallInfo fcinfo, /* * If there's just one OUT parameter, out_param_varno points * directly to it. If there's more than one, build a row that - * holds all of them. + * holds all of them. Procedures return a row even for one OUT + * parameter. */ - if (num_out_args == 1) - function->out_param_varno = out_arg_variables[0]->dno; - else if (num_out_args > 1) + if (num_out_args > 1 || + (num_out_args == 1 && function->fn_prokind == PROKIND_PROCEDURE)) { PLpgSQL_row *row = build_row_from_vars(out_arg_variables, num_out_args); @@ -487,6 +487,8 @@ do_compile(FunctionCallInfo fcinfo, plpgsql_adddatum((PLpgSQL_datum *) row); function->out_param_varno = row->dno; } + else if (num_out_args == 1) + function->out_param_varno = out_arg_variables[0]->dno; /* * Check for a polymorphic returntype. If found, use the actual diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 489484f184..0841a81e87 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -24,6 +24,7 @@ #include "catalog/pg_type.h" #include "executor/execExpr.h" #include "executor/spi.h" +#include "executor/spi_priv.h" #include "funcapi.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" @@ -40,6 +41,7 @@ #include "utils/memutils.h" #include "utils/rel.h" #include "utils/snapmgr.h" +#include "utils/syscache.h" #include "utils/typcache.h" #include "plpgsql.h" @@ -253,6 +255,8 @@ static int exec_stmt_assign(PLpgSQL_execstate *estate, PLpgSQL_stmt_assign *stmt); static int exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt); +static int exec_stmt_call(PLpgSQL_execstate *estate, + PLpgSQL_stmt_call *stmt); static int exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt); static int exec_stmt_if(PLpgSQL_execstate *estate, @@ -1901,6 +1905,10 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt) rc = exec_stmt_perform(estate, (PLpgSQL_stmt_perform *) stmt); break; + case PLPGSQL_STMT_CALL: + rc = exec_stmt_call(estate, (PLpgSQL_stmt_call *) stmt); + break; + case PLPGSQL_STMT_GETDIAG: rc = exec_stmt_getdiag(estate, (PLpgSQL_stmt_getdiag *) stmt); break; @@ -2041,6 +2049,121 @@ exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt) return PLPGSQL_RC_OK; } +/* + * exec_stmt_call + */ +static int +exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) +{ + PLpgSQL_expr *expr = stmt->expr; + ParamListInfo paramLI; + int rc; + + if (expr->plan == NULL) + exec_prepare_plan(estate, expr, 0); + + paramLI = setup_param_list(estate, expr); + + rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI, + estate->readonly_func, 0); + + if (rc < 0) + elog(ERROR, "SPI_execute_plan_with_paramlist failed executing query \"%s\": %s", + expr->query, SPI_result_code_string(rc)); + + if (SPI_processed == 1) + { + SPITupleTable *tuptab = SPI_tuptable; + + /* + * Construct a dummy target row based on the INOUT parameters of the + * procedure call. + */ + if (!stmt->target) + { + Node *node; + ListCell *lc; + FuncExpr *funcexpr; + int i; + HeapTuple tuple; + int numargs; + Oid *argtypes; + char **argnames; + char *argmodes; + MemoryContext oldcontext; + PLpgSQL_row *row; + int nfields; + + /* + * Get the original CallStmt + */ + node = linitial_node(Query, ((CachedPlanSource *) linitial(expr->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 parameter modes + */ + tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcexpr->funcid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for function %u", funcexpr->funcid); + numargs = get_func_arg_info(tuple, &argtypes, &argnames, &argmodes); + ReleaseSysCache(tuple); + + Assert(numargs == list_length(funcexpr->args)); + + /* + * Construct row + */ + oldcontext = MemoryContextSwitchTo(estate->func->fn_cxt); + + row = palloc0(sizeof(*row)); + row->dtype = PLPGSQL_DTYPE_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) + { + Param *param; + + if (!IsA(n, Param)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("argument %d is an output parameter but is not writable", i + 1))); + + param = castNode(Param, n); + /* paramid is offset by 1 (see make_datum_param()) */ + row->varnos[nfields++] = param->paramid - 1; + } + i++; + } + + row->nfields = nfields; + + MemoryContextSwitchTo(oldcontext); + + stmt->target = (PLpgSQL_variable *) row; + } + + exec_move_row(estate, stmt->target, tuptab->vals[0], tuptab->tupdesc); + } + else if (SPI_processed > 1) + elog(ERROR, "procedure call returned more than one row"); + + exec_eval_cleanup(estate); + SPI_freetuptable(SPI_tuptable); + + return PLPGSQL_RC_OK; +} + /* ---------- * exec_stmt_getdiag Put internal PG information into * specified variables. @@ -6763,7 +6886,7 @@ exec_move_row_from_fields(PLpgSQL_execstate *estate, return; } - elog(ERROR, "unsupported target"); + elog(ERROR, "unsupported target type: %d", target->dtype); } /* diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c index b986fc39b3..39d6a54663 100644 --- a/src/pl/plpgsql/src/pl_funcs.c +++ b/src/pl/plpgsql/src/pl_funcs.c @@ -284,6 +284,8 @@ plpgsql_stmt_typename(PLpgSQL_stmt *stmt) return "CLOSE"; case PLPGSQL_STMT_PERFORM: return "PERFORM"; + case PLPGSQL_STMT_CALL: + return "CALL"; case PLPGSQL_STMT_COMMIT: return "COMMIT"; case PLPGSQL_STMT_ROLLBACK: @@ -367,6 +369,7 @@ static void free_open(PLpgSQL_stmt_open *stmt); static void free_fetch(PLpgSQL_stmt_fetch *stmt); static void free_close(PLpgSQL_stmt_close *stmt); static void free_perform(PLpgSQL_stmt_perform *stmt); +static void free_call(PLpgSQL_stmt_call *stmt); static void free_commit(PLpgSQL_stmt_commit *stmt); static void free_rollback(PLpgSQL_stmt_rollback *stmt); static void free_expr(PLpgSQL_expr *expr); @@ -449,6 +452,9 @@ free_stmt(PLpgSQL_stmt *stmt) case PLPGSQL_STMT_PERFORM: free_perform((PLpgSQL_stmt_perform *) stmt); break; + case PLPGSQL_STMT_CALL: + free_call((PLpgSQL_stmt_call *) stmt); + break; case PLPGSQL_STMT_COMMIT: free_commit((PLpgSQL_stmt_commit *) stmt); break; @@ -602,6 +608,12 @@ free_perform(PLpgSQL_stmt_perform *stmt) free_expr(stmt->expr); } +static void +free_call(PLpgSQL_stmt_call *stmt) +{ + free_expr(stmt->expr); +} + static void free_commit(PLpgSQL_stmt_commit *stmt) { @@ -805,6 +817,7 @@ static void dump_fetch(PLpgSQL_stmt_fetch *stmt); static void dump_cursor_direction(PLpgSQL_stmt_fetch *stmt); static void dump_close(PLpgSQL_stmt_close *stmt); static void dump_perform(PLpgSQL_stmt_perform *stmt); +static void dump_call(PLpgSQL_stmt_call *stmt); static void dump_commit(PLpgSQL_stmt_commit *stmt); static void dump_rollback(PLpgSQL_stmt_rollback *stmt); static void dump_expr(PLpgSQL_expr *expr); @@ -897,6 +910,9 @@ dump_stmt(PLpgSQL_stmt *stmt) case PLPGSQL_STMT_PERFORM: dump_perform((PLpgSQL_stmt_perform *) stmt); break; + case PLPGSQL_STMT_CALL: + dump_call((PLpgSQL_stmt_call *) stmt); + break; case PLPGSQL_STMT_COMMIT: dump_commit((PLpgSQL_stmt_commit *) stmt); break; @@ -1275,6 +1291,15 @@ dump_perform(PLpgSQL_stmt_perform *stmt) printf("\n"); } +static void +dump_call(PLpgSQL_stmt_call *stmt) +{ + dump_ind(); + printf("CALL expr = "); + dump_expr(stmt->expr); + printf("\n"); +} + static void dump_commit(PLpgSQL_stmt_commit *stmt) { diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index 9fcf2424da..d69e66d00a 100644 --- a/src/pl/plpgsql/src/pl_gram.y +++ b/src/pl/plpgsql/src/pl_gram.y @@ -197,7 +197,7 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt); %type proc_stmt pl_block %type stmt_assign stmt_if stmt_loop stmt_while stmt_exit %type stmt_return stmt_raise stmt_assert stmt_execsql -%type stmt_dynexecute stmt_for stmt_perform stmt_getdiag +%type stmt_dynexecute stmt_for stmt_perform stmt_call stmt_getdiag %type stmt_open stmt_fetch stmt_move stmt_close stmt_null %type stmt_commit stmt_rollback %type stmt_case stmt_foreach_a @@ -257,6 +257,7 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt); %token K_BACKWARD %token K_BEGIN %token K_BY +%token K_CALL %token K_CASE %token K_CLOSE %token K_COLLATE @@ -872,6 +873,8 @@ proc_stmt : pl_block ';' { $$ = $1; } | stmt_perform { $$ = $1; } + | stmt_call + { $$ = $1; } | stmt_getdiag { $$ = $1; } | stmt_open @@ -903,6 +906,20 @@ stmt_perform : K_PERFORM expr_until_semi } ; +stmt_call : K_CALL + { + PLpgSQL_stmt_call *new; + + new = palloc0(sizeof(PLpgSQL_stmt_call)); + new->cmd_type = PLPGSQL_STMT_CALL; + new->lineno = plpgsql_location_to_lineno(@1); + new->expr = read_sql_stmt("CALL "); + + $$ = (PLpgSQL_stmt *)new; + + } + ; + stmt_assign : assign_var assign_operator expr_until_semi { PLpgSQL_stmt_assign *new; @@ -2401,6 +2418,7 @@ unreserved_keyword : | K_ARRAY | K_ASSERT | K_BACKWARD + | K_CALL | K_CLOSE | K_COLLATE | K_COLUMN @@ -3129,15 +3147,6 @@ make_return_stmt(int location) errhint("Use RETURN NEXT or RETURN QUERY."), parser_errposition(yylloc))); } - else if (plpgsql_curr_compile->out_param_varno >= 0) - { - if (yylex() != ';') - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("RETURN cannot have a parameter in function with OUT parameters"), - parser_errposition(yylloc))); - new->retvarno = plpgsql_curr_compile->out_param_varno; - } else if (plpgsql_curr_compile->fn_rettype == VOIDOID) { if (yylex() != ';') @@ -3154,6 +3163,15 @@ make_return_stmt(int location) parser_errposition(yylloc))); } } + else if (plpgsql_curr_compile->out_param_varno >= 0) + { + if (yylex() != ';') + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("RETURN cannot have a parameter in function with OUT parameters"), + parser_errposition(yylloc))); + new->retvarno = plpgsql_curr_compile->out_param_varno; + } else { /* diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c index 12a3e6b818..65774f9902 100644 --- a/src/pl/plpgsql/src/pl_scanner.c +++ b/src/pl/plpgsql/src/pl_scanner.c @@ -102,6 +102,7 @@ static const ScanKeyword unreserved_keywords[] = { PG_KEYWORD("array", K_ARRAY, UNRESERVED_KEYWORD) PG_KEYWORD("assert", K_ASSERT, UNRESERVED_KEYWORD) PG_KEYWORD("backward", K_BACKWARD, UNRESERVED_KEYWORD) + PG_KEYWORD("call", K_CALL, UNRESERVED_KEYWORD) PG_KEYWORD("close", K_CLOSE, UNRESERVED_KEYWORD) PG_KEYWORD("collate", K_COLLATE, UNRESERVED_KEYWORD) PG_KEYWORD("column", K_COLUMN, UNRESERVED_KEYWORD) diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index dd59036de0..f7619a63f9 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -125,6 +125,7 @@ typedef enum PLpgSQL_stmt_type PLPGSQL_STMT_FETCH, PLPGSQL_STMT_CLOSE, PLPGSQL_STMT_PERFORM, + PLPGSQL_STMT_CALL, PLPGSQL_STMT_COMMIT, PLPGSQL_STMT_ROLLBACK } PLpgSQL_stmt_type; @@ -508,6 +509,17 @@ typedef struct PLpgSQL_stmt_perform PLpgSQL_expr *expr; } PLpgSQL_stmt_perform; +/* + * CALL statement + */ +typedef struct PLpgSQL_stmt_call +{ + PLpgSQL_stmt_type cmd_type; + int lineno; + PLpgSQL_expr *expr; + PLpgSQL_variable *target; +} PLpgSQL_stmt_call; + /* * COMMIT statement */ diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql index e580e5fea0..1fc13ddc5e 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_call.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql @@ -55,6 +55,114 @@ CREATE PROCEDURE test_proc4(y int) SELECT * FROM test1; +-- OUT parameters + +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE plpgsql +AS $$ +BEGIN + a := a || '+' || a; +END; +$$; + +CALL test_proc5('abc'); + + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE plpgsql +AS $$ +BEGIN + b := b * a; + c := c * a; +END; +$$; + +CALL test_proc6(2, 3, 4); + + +DO +LANGUAGE plpgsql +$$ +DECLARE + x int := 3; + y int := 4; +BEGIN + CALL test_proc6(2, x, y); + RAISE INFO 'x = %, y = %', x, y; +END; +$$; + + +DO +LANGUAGE plpgsql +$$ +DECLARE + x int := 3; + y int := 4; +BEGIN + CALL test_proc6(2, x + 1, y); -- error + RAISE INFO 'x = %, y = %', x, y; +END; +$$; + + +DO +LANGUAGE plpgsql +$$ +DECLARE + x int := 3; + y int := 4; +BEGIN + FOR i IN 1..5 LOOP + CALL test_proc6(i, x, y); + RAISE INFO 'x = %, y = %', x, y; + END LOOP; +END; +$$; + + +-- recursive with output parameters + +CREATE PROCEDURE test_proc7(x int, INOUT a int, INOUT b numeric) +LANGUAGE plpgsql +AS $$ +BEGIN +IF x > 1 THEN + a := x / 10; + b := x / 2; + CALL test_proc7(b::int, a, b); +END IF; +END; +$$; + +CALL test_proc7(100, -1, -1); + + +-- transition variable assignment + +TRUNCATE test1; + +CREATE FUNCTION f() RETURNS trigger +LANGUAGE plpgsql +AS $$ +DECLARE + z int := 0; +BEGIN + --NEW.a := NEW.a * 2; + CALL test_proc6(2, NEW.a, NEW.a); + RETURN NEW; +END; +$$; + +CREATE TRIGGER t BEFORE INSERT ON test1 EXECUTE PROCEDURE f(); + +INSERT INTO test1 VALUES (1), (2), (3); + +UPDATE test1 SET a = 22 WHERE a = 2; + +SELECT * FROM test1 ORDER BY a; + + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc3; DROP PROCEDURE test_proc4; diff --git a/src/pl/plpython/expected/plpython_call.out b/src/pl/plpython/expected/plpython_call.out index 90785343b6..dff15743aa 100644 --- a/src/pl/plpython/expected/plpython_call.out +++ b/src/pl/plpython/expected/plpython_call.out @@ -29,6 +29,29 @@ SELECT * FROM test1; 55 (1 row) +-- OUT parameters +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE plpythonu +AS $$ +return [a + '+' + a] +$$; +CALL test_proc5('abc'); + a +--------- + abc+abc +(1 row) + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE plpythonu +AS $$ +return (b * a, c * a) +$$; +CALL test_proc6(2, 3, 4); + b | c +---+--- + 6 | 8 +(1 row) + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; DROP PROCEDURE test_proc3; diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c index 1e0f3d9d3a..7c8c7dee87 100644 --- a/src/pl/plpython/plpy_exec.c +++ b/src/pl/plpython/plpy_exec.c @@ -204,21 +204,19 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) * return value as a special "void datum" rather than NULL (as is the * case for non-void-returning functions). */ - if (proc->is_procedure) + if (proc->result.typoid == VOIDOID) { if (plrv != Py_None) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("PL/Python procedure did not return None"))); - fcinfo->isnull = false; - rv = (Datum) 0; - } - else if (proc->result.typoid == VOIDOID) - { - if (plrv != Py_None) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("PL/Python function with return type \"void\" did not return None"))); + { + if (proc->is_procedure) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("PL/Python procedure did not return None"))); + else + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("PL/Python function with return type \"void\" did not return None"))); + } fcinfo->isnull = false; rv = (Datum) 0; diff --git a/src/pl/plpython/sql/plpython_call.sql b/src/pl/plpython/sql/plpython_call.sql index 3fb74de5f0..b71ab284dc 100644 --- a/src/pl/plpython/sql/plpython_call.sql +++ b/src/pl/plpython/sql/plpython_call.sql @@ -34,6 +34,26 @@ CREATE PROCEDURE test_proc3(x int) SELECT * FROM test1; +-- OUT parameters + +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE plpythonu +AS $$ +return [a + '+' + a] +$$; + +CALL test_proc5('abc'); + + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE plpythonu +AS $$ +return (b * a, c * a) +$$; + +CALL test_proc6(2, 3, 4); + + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; DROP PROCEDURE test_proc3; diff --git a/src/pl/tcl/expected/pltcl_call.out b/src/pl/tcl/expected/pltcl_call.out index 7221a37ad0..db286522c4 100644 --- a/src/pl/tcl/expected/pltcl_call.out +++ b/src/pl/tcl/expected/pltcl_call.out @@ -23,6 +23,32 @@ SELECT * FROM test1; 55 (1 row) +-- OUT parameters +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE pltcl +AS $$ +set aa [concat $1 "+" $1] +return [list a $aa] +$$; +CALL test_proc5('abc'); + a +----------- + abc + abc +(1 row) + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE pltcl +AS $$ +set bb [expr $2 * $1] +set cc [expr $3 * $1] +return [list b $bb c $cc] +$$; +CALL test_proc6(2, 3, 4); + b | c +---+--- + 6 | 8 +(1 row) + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; DROP PROCEDURE test_proc3; diff --git a/src/pl/tcl/sql/pltcl_call.sql b/src/pl/tcl/sql/pltcl_call.sql index ef1f540f50..d6e350872d 100644 --- a/src/pl/tcl/sql/pltcl_call.sql +++ b/src/pl/tcl/sql/pltcl_call.sql @@ -29,6 +29,29 @@ CREATE PROCEDURE test_proc3(x int) SELECT * FROM test1; +-- OUT parameters + +CREATE PROCEDURE test_proc5(INOUT a text) +LANGUAGE pltcl +AS $$ +set aa [concat $1 "+" $1] +return [list a $aa] +$$; + +CALL test_proc5('abc'); + + +CREATE PROCEDURE test_proc6(a int, INOUT b int, INOUT c int) +LANGUAGE pltcl +AS $$ +set bb [expr $2 * $1] +set cc [expr $3 * $1] +return [list b $bb c $cc] +$$; + +CALL test_proc6(2, 3, 4); + + DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; DROP PROCEDURE test_proc3; diff --git a/src/test/regress/expected/create_procedure.out b/src/test/regress/expected/create_procedure.out index 182b325ea1..6f217a674f 100644 --- a/src/test/regress/expected/create_procedure.out +++ b/src/test/regress/expected/create_procedure.out @@ -71,6 +71,26 @@ SELECT * FROM cp_test; 1 | b (2 rows) +-- output parameters +CREATE PROCEDURE ptest4a(INOUT a int, INOUT b int) +LANGUAGE SQL +AS $$ +SELECT 1, 2; +$$; +CALL ptest4a(NULL, NULL); + a | b +---+--- + 1 | 2 +(1 row) + +CREATE PROCEDURE ptest4b(INOUT b int, INOUT a int) +LANGUAGE SQL +AS $$ +CALL ptest4a(a, b); -- error, not supported +$$; +ERROR: calling procedures with output parameters is not supported in SQL functions +CONTEXT: SQL function "ptest4b" +DROP PROCEDURE ptest4a; -- various error cases CALL version(); -- error: not a procedure ERROR: version() is not a procedure @@ -91,6 +111,7 @@ LINE 1: CREATE PROCEDURE ptestx() LANGUAGE SQL STRICT AS $$ INSERT I... ^ CREATE PROCEDURE ptestx(OUT a int) LANGUAGE SQL AS $$ INSERT INTO cp_test VALUES (1, 'a') $$; ERROR: procedures cannot have OUT parameters +HINT: INOUT parameters are permitted. ALTER PROCEDURE ptest1(text) STRICT; ERROR: invalid attribute in procedure definition LINE 1: ALTER PROCEDURE ptest1(text) STRICT; diff --git a/src/test/regress/sql/create_procedure.sql b/src/test/regress/sql/create_procedure.sql index 52318bf2a6..31b1db98f5 100644 --- a/src/test/regress/sql/create_procedure.sql +++ b/src/test/regress/sql/create_procedure.sql @@ -46,6 +46,25 @@ CREATE PROCEDURE ptest3(y text) SELECT * FROM cp_test; +-- output parameters + +CREATE PROCEDURE ptest4a(INOUT a int, INOUT b int) +LANGUAGE SQL +AS $$ +SELECT 1, 2; +$$; + +CALL ptest4a(NULL, NULL); + +CREATE PROCEDURE ptest4b(INOUT b int, INOUT a int) +LANGUAGE SQL +AS $$ +CALL ptest4a(a, b); -- error, not supported +$$; + +DROP PROCEDURE ptest4a; + + -- various error cases CALL version(); -- error: not a procedure base-commit: 4a4e2442a7f7c1434e86dd290cdb3704cfebb24c -- 2.16.2