Here's a stripped-down patch that drops the change in what should be in CALL argument lists, and just focuses on reverting the change in pg_proc.proargtypes and the consequent mess for ALTER/DROP ROUTINE. I spent some more effort on the docs, too.
regards, tom lane
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 16493209c6..f517a7d4af 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -5905,9 +5905,8 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l <para> An array of the data types of the function arguments. This includes only input arguments (including <literal>INOUT</literal> and - <literal>VARIADIC</literal> arguments), as well as - <literal>OUT</literal> parameters of procedures, and thus represents - the call signature of the function or procedure. + <literal>VARIADIC</literal> arguments), and thus represents + the call signature of the function. </para></entry> </row> diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 52f60c827c..0cd6e8b071 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -480,7 +480,7 @@ $$ LANGUAGE plpgsql; <para> To call a function with <literal>OUT</literal> parameters, omit the - output parameter in the function call: + output parameter(s) in the function call: <programlisting> SELECT sales_tax(100.00); </programlisting> @@ -523,16 +523,20 @@ $$ LANGUAGE plpgsql; </programlisting> In a call to a procedure, all the parameters must be specified. For - output parameters, <literal>NULL</literal> may be specified. + output parameters, <literal>NULL</literal> may be specified when + calling the procedure from plain SQL: <programlisting> CALL sum_n_product(2, 4, NULL, NULL); sum | prod -----+------ 6 | 8 </programlisting> - Output parameters in procedures become more interesting in nested calls, - where they can be assigned to variables. See <xref - linkend="plpgsql-statements-calling-procedure"/> for details. + + However, when calling a procedure + from <application>PL/pgSQL</application>, you should instead write a + variable for any output parameter; the variable will receive the result + of the call. See <xref linkend="plpgsql-statements-calling-procedure"/> + for details. </para> <para> @@ -2030,6 +2034,9 @@ BEGIN END; $$; </programlisting> + The variable corresponding to an output parameter can be a simple + variable or a field of a composite-type variable. Currently, + it cannot be an element of an array. </para> </sect2> diff --git a/doc/src/sgml/ref/alter_extension.sgml b/doc/src/sgml/ref/alter_extension.sgml index 38fd60128b..c819c7bb4e 100644 --- a/doc/src/sgml/ref/alter_extension.sgml +++ b/doc/src/sgml/ref/alter_extension.sgml @@ -212,12 +212,11 @@ ALTER EXTENSION <replaceable class="parameter">name</replaceable> DROP <replacea argument: <literal>IN</literal>, <literal>OUT</literal>, <literal>INOUT</literal>, or <literal>VARIADIC</literal>. If omitted, the default is <literal>IN</literal>. - Note that <command>ALTER EXTENSION</command> does not actually pay any - attention to <literal>OUT</literal> arguments for functions and - aggregates (but not procedures), since only the input arguments are - needed to determine the function's identity. So it is sufficient to - list the <literal>IN</literal>, <literal>INOUT</literal>, and - <literal>VARIADIC</literal> arguments for functions and aggregates. + Note that <command>ALTER EXTENSION</command> does not actually pay + any attention to <literal>OUT</literal> arguments, since only the input + arguments are needed to determine the function's identity. + So it is sufficient to list the <literal>IN</literal>, <literal>INOUT</literal>, + and <literal>VARIADIC</literal> arguments. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/call.sgml b/doc/src/sgml/ref/call.sgml index abaa81c78b..dbad89bbf7 100644 --- a/doc/src/sgml/ref/call.sgml +++ b/doc/src/sgml/ref/call.sgml @@ -55,9 +55,21 @@ CALL <replaceable class="parameter">name</replaceable> ( [ <replaceable class="p <term><replaceable class="parameter">argument</replaceable></term> <listitem> <para> - 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. + An argument expression for the procedure call. + </para> + + <para> + Arguments can include parameter names, using the syntax + <literal><replaceable class="parameter">name</replaceable> => <replaceable class="parameter">value</replaceable></literal>. + This works the same as in ordinary function calls; see + <xref linkend="sql-syntax-calling-funcs"/> for details. + </para> + + <para> + Arguments must be supplied for all procedure parameters that lack + defaults, including <literal>OUT</literal> parameters. However, + arguments matching <literal>OUT</literal> parameters are not evaluated, + so it's customary to just write <literal>NULL</literal> for them. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml index eda91b4e24..6e8ced3eaf 100644 --- a/doc/src/sgml/ref/comment.sgml +++ b/doc/src/sgml/ref/comment.sgml @@ -178,12 +178,11 @@ COMMENT ON argument: <literal>IN</literal>, <literal>OUT</literal>, <literal>INOUT</literal>, or <literal>VARIADIC</literal>. If omitted, the default is <literal>IN</literal>. - Note that <command>COMMENT</command> does not actually pay any attention - to <literal>OUT</literal> arguments for functions and aggregates (but - not procedures), since only the input arguments are needed to determine - the function's identity. So it is sufficient to list the - <literal>IN</literal>, <literal>INOUT</literal>, and - <literal>VARIADIC</literal> arguments for functions and aggregates. + Note that <command>COMMENT</command> does not actually pay + any attention to <literal>OUT</literal> arguments, since only the input + arguments are needed to determine the function's identity. + So it is sufficient to list the <literal>IN</literal>, <literal>INOUT</literal>, + and <literal>VARIADIC</literal> arguments. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/security_label.sgml b/doc/src/sgml/ref/security_label.sgml index 9b87bcd519..e9688cce21 100644 --- a/doc/src/sgml/ref/security_label.sgml +++ b/doc/src/sgml/ref/security_label.sgml @@ -127,12 +127,11 @@ SECURITY LABEL [ FOR <replaceable class="parameter">provider</replaceable> ] ON argument: <literal>IN</literal>, <literal>OUT</literal>, <literal>INOUT</literal>, or <literal>VARIADIC</literal>. If omitted, the default is <literal>IN</literal>. - Note that <command>SECURITY LABEL</command> does not actually pay any - attention to <literal>OUT</literal> arguments for functions and - aggregates (but not procedures), since only the input arguments are - needed to determine the function's identity. So it is sufficient to - list the <literal>IN</literal>, <literal>INOUT</literal>, and - <literal>VARIADIC</literal> arguments for functions and aggregates. + Note that <command>SECURITY LABEL</command> does not actually + pay any attention to <literal>OUT</literal> arguments, since only the input + arguments are needed to determine the function's identity. + So it is sufficient to list the <literal>IN</literal>, <literal>INOUT</literal>, + and <literal>VARIADIC</literal> arguments. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 41bcc5b79d..3771401c01 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -765,7 +765,7 @@ DROP FUNCTION sum_n_product (int, int); parameter serves as both an input parameter (part of the calling argument list) and an output parameter (part of the result record type). <literal>VARIADIC</literal> parameters are input parameters, but are treated - specially as described next. + specially as described below. </para> </sect2> @@ -779,12 +779,8 @@ DROP FUNCTION sum_n_product (int, int); <para> Output parameters are also supported in procedures, but they work a bit - differently from functions. Notably, output parameters - <emphasis>are</emphasis> included in the signature of a procedure and - must be specified in the procedure call. - </para> - - <para> + differently from functions. In <command>CALL</command> commands, + output parameters must be included in the argument list. For example, the bank account debiting routine from earlier could be written like this: <programlisting> @@ -795,17 +791,21 @@ CREATE PROCEDURE tp1 (accountno integer, debit numeric, OUT new_balance numeric) RETURNING balance; $$ LANGUAGE SQL; </programlisting> - To call this procedure, it is irrelevant what is passed as the argument - of the <literal>OUT</literal> parameter, so you could pass + To call this procedure, an argument matching the <literal>OUT</literal> + parameter must be included. It's customary to write <literal>NULL</literal>: <programlisting> CALL tp1(17, 100.0, NULL); </programlisting> + If you write something else, it must be an expression that is implicitly + coercible to the declared type of the parameter, just as for input + parameters. Note however that such an expression will not be evaluated. </para> <para> - Procedures with output parameters are more useful in PL/pgSQL, where the - output parameters can be assigned to variables. See <xref + When calling a procedure from <application>PL/pgSQL</application>, + instead of writing <literal>NULL</literal> you must write a variable + that will receive the procedure's output. See <xref linkend="plpgsql-statements-calling-procedure"/> for details. </para> </sect2> diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 005e029c38..fd767fc5cf 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -206,6 +206,7 @@ static void RemoveTempRelations(Oid tempNamespaceId); static void RemoveTempRelationsCallback(int code, Datum arg); static void NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue); static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames, + bool include_out_arguments, int pronargs, int **argnumbers); @@ -901,6 +902,12 @@ TypeIsVisible(Oid typid) * of additional args (which can be retrieved from the function's * proargdefaults entry). * + * If include_out_arguments is true, then OUT-mode arguments are considered to + * be included in the argument list. Their types are included in the returned + * arrays, and argnumbers are indexes in proallargtypes not proargtypes. + * We also set nominalnargs to be the length of proallargtypes not proargtypes. + * Otherwise OUT-mode arguments are ignored. + * * It is not possible for nvargs and ndargs to both be nonzero in the same * list entry, since default insertion allows matches to functions with more * than nargs arguments while the variadic transformation requires the same @@ -911,7 +918,8 @@ TypeIsVisible(Oid typid) * first any positional arguments, then the named arguments, then defaulted * arguments (if needed and allowed by expand_defaults). The argnumbers[] * array can be used to map this back to the catalog information. - * argnumbers[k] is set to the proargtypes index of the k'th call argument. + * argnumbers[k] is set to the proargtypes or proallargtypes index of the + * k'th call argument. * * We search a single namespace if the function name is qualified, else * all namespaces in the search path. In the multiple-namespace case, @@ -935,13 +943,13 @@ TypeIsVisible(Oid typid) * such an entry it should react as though the call were ambiguous. * * If missing_ok is true, an empty list (NULL) is returned if the name was - * schema- qualified with a schema that does not exist. Likewise if no + * schema-qualified with a schema that does not exist. Likewise if no * candidate is found for other reasons. */ FuncCandidateList FuncnameGetCandidates(List *names, int nargs, List *argnames, bool expand_variadic, bool expand_defaults, - bool missing_ok) + bool include_out_arguments, bool missing_ok) { FuncCandidateList resultList = NULL; bool any_special = false; @@ -978,6 +986,7 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, { HeapTuple proctup = &catlist->members[i]->tuple; Form_pg_proc procform = (Form_pg_proc) GETSTRUCT(proctup); + Oid *proargtypes = procform->proargtypes.values; int pronargs = procform->pronargs; int effective_nargs; int pathpos = 0; @@ -1012,6 +1021,35 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, continue; /* proc is not in search path */ } + /* + * If we are asked to match to OUT arguments, then use the + * proallargtypes array (which includes those); otherwise use + * proargtypes (which doesn't). Of course, if proallargtypes is null, + * we always use proargtypes. + */ + if (include_out_arguments) + { + Datum proallargtypes; + bool isNull; + + proallargtypes = SysCacheGetAttr(PROCNAMEARGSNSP, proctup, + Anum_pg_proc_proallargtypes, + &isNull); + if (!isNull) + { + ArrayType *arr = DatumGetArrayTypeP(proallargtypes); + + pronargs = ARR_DIMS(arr)[0]; + if (ARR_NDIM(arr) != 1 || + pronargs < 0 || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != OIDOID) + elog(ERROR, "proallargtypes is not a 1-D Oid array or it contains nulls"); + Assert(pronargs >= procform->pronargs); + proargtypes = (Oid *) ARR_DATA_PTR(arr); + } + } + if (argnames != NIL) { /* @@ -1047,6 +1085,7 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, /* Check for argument name match, generate positional mapping */ if (!MatchNamedCall(proctup, nargs, argnames, + include_out_arguments, pronargs, &argnumbers)) continue; @@ -1105,12 +1144,12 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, effective_nargs * sizeof(Oid)); newResult->pathpos = pathpos; newResult->oid = procform->oid; + newResult->nominalnargs = pronargs; newResult->nargs = effective_nargs; newResult->argnumbers = argnumbers; if (argnumbers) { /* Re-order the argument types into call's logical order */ - Oid *proargtypes = procform->proargtypes.values; int i; for (i = 0; i < pronargs; i++) @@ -1119,8 +1158,7 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, else { /* Simple positional case, just copy proargtypes as-is */ - memcpy(newResult->args, procform->proargtypes.values, - pronargs * sizeof(Oid)); + memcpy(newResult->args, proargtypes, pronargs * sizeof(Oid)); } if (variadic) { @@ -1293,6 +1331,10 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, * the function, in positions after the last positional argument, and there * are defaults for all unsupplied arguments. * + * If include_out_arguments is true, we are treating OUT arguments as + * included in the argument list. pronargs is the number of arguments + * we're considering (the length of either proargtypes or proallargtypes). + * * The number of positional arguments is nargs - list_length(argnames). * Note caller has already done basic checks on argument count. * @@ -1303,10 +1345,10 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, */ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames, + bool include_out_arguments, int pronargs, int **argnumbers) { Form_pg_proc procform = (Form_pg_proc) GETSTRUCT(proctup); - int pronargs = procform->pronargs; int numposargs = nargs - list_length(argnames); int pronallargs; Oid *p_argtypes; @@ -1333,6 +1375,8 @@ MatchNamedCall(HeapTuple proctup, int nargs, List *argnames, &p_argtypes, &p_argnames, &p_argmodes); Assert(p_argnames != NULL); + Assert(include_out_arguments ? (pronargs == pronallargs) : (pronargs <= pronallargs)); + /* initialize state for matching */ *argnumbers = (int *) palloc(pronargs * sizeof(int)); memset(arggiven, false, pronargs * sizeof(bool)); @@ -1355,8 +1399,9 @@ MatchNamedCall(HeapTuple proctup, int nargs, List *argnames, found = false; for (i = 0; i < pronallargs; i++) { - /* consider only input parameters */ - if (p_argmodes && + /* consider only input params, except with include_out_arguments */ + if (!include_out_arguments && + p_argmodes && (p_argmodes[i] != FUNC_PARAM_IN && p_argmodes[i] != FUNC_PARAM_INOUT && p_argmodes[i] != FUNC_PARAM_VARIADIC)) @@ -1371,7 +1416,7 @@ MatchNamedCall(HeapTuple proctup, int nargs, List *argnames, found = true; break; } - /* increase pp only for input parameters */ + /* increase pp only for considered parameters */ pp++; } /* if name isn't in proargnames, fail */ @@ -1448,7 +1493,7 @@ FunctionIsVisible(Oid funcid) visible = false; clist = FuncnameGetCandidates(list_make1(makeString(proname)), - nargs, NIL, false, false, false); + nargs, NIL, false, false, false, false); for (; clist; clist = clist->next) { @@ -1721,6 +1766,7 @@ OpernameGetCandidates(List *names, char oprkind, bool missing_schema_ok) newResult->pathpos = pathpos; newResult->oid = operform->oid; + newResult->nominalnargs = 2; newResult->nargs = 2; newResult->nvargs = 0; newResult->ndargs = 0; diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 5197076c76..1f63d8081b 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -846,7 +846,7 @@ lookup_agg_function(List *fnName, * the function. */ fdresult = func_get_detail(fnName, NIL, NIL, - nargs, input_types, false, false, + nargs, input_types, false, false, false, &fnOid, rettype, &retset, &nvargs, &vatype, &true_oid_array, NULL); diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 5403110820..1454d2fb67 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -471,12 +471,10 @@ ProcedureCreate(const char *procedureName, if (isnull) proargmodes = PointerGetDatum(NULL); /* just to be sure */ - n_old_arg_names = get_func_input_arg_names(prokind, - proargnames, + n_old_arg_names = get_func_input_arg_names(proargnames, proargmodes, &old_arg_names); - n_new_arg_names = get_func_input_arg_names(prokind, - parameterNames, + n_new_arg_names = get_func_input_arg_names(parameterNames, parameterModes, &new_arg_names); for (j = 0; j < n_old_arg_names; j++) diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 4c12aa33df..8572d8543a 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -169,16 +169,16 @@ compute_return_type(TypeName *returnType, Oid languageOid, } /* - * Interpret the function parameter list of a CREATE FUNCTION or - * CREATE AGGREGATE statement. + * Interpret the function parameter list of a CREATE FUNCTION, + * CREATE PROCEDURE, or CREATE AGGREGATE statement. * * Input parameters: * parameters: list of FunctionParameter structs * languageOid: OID of function language (InvalidOid if it's CREATE AGGREGATE) - * objtype: needed only to determine error handling and required result type + * objtype: identifies type of object being created * * Results are stored into output parameters. parameterTypes must always - * be created, but the other arrays are set to NULL if not needed. + * be created, but the other arrays/lists can be NULL pointers if not needed. * variadicArgType is set to the variadic array type if there's a VARIADIC * parameter (there can be only one); or to InvalidOid if not. * requiredResultType is set to InvalidOid if there are no OUT parameters, @@ -200,8 +200,8 @@ interpret_function_parameter_list(ParseState *pstate, Oid *requiredResultType) { int parameterCount = list_length(parameters); - Oid *sigArgTypes; - int sigArgCount = 0; + Oid *inTypes; + int inCount = 0; Datum *allTypes; Datum *paramModes; Datum *paramNames; @@ -215,7 +215,7 @@ interpret_function_parameter_list(ParseState *pstate, *variadicArgType = InvalidOid; /* default result */ *requiredResultType = InvalidOid; /* default result */ - sigArgTypes = (Oid *) palloc(parameterCount * sizeof(Oid)); + inTypes = (Oid *) palloc(parameterCount * sizeof(Oid)); allTypes = (Datum *) palloc(parameterCount * sizeof(Datum)); paramModes = (Datum *) palloc(parameterCount * sizeof(Datum)); paramNames = (Datum *) palloc0(parameterCount * sizeof(Datum)); @@ -290,29 +290,34 @@ interpret_function_parameter_list(ParseState *pstate, /* handle input parameters */ if (fp->mode != FUNC_PARAM_OUT && fp->mode != FUNC_PARAM_TABLE) { - isinput = true; - if (parameterTypes_list) - *parameterTypes_list = lappend_oid(*parameterTypes_list, toid); - } - - /* handle signature parameters */ - if (fp->mode == FUNC_PARAM_IN || fp->mode == FUNC_PARAM_INOUT || - (objtype == OBJECT_PROCEDURE && fp->mode == FUNC_PARAM_OUT) || - fp->mode == FUNC_PARAM_VARIADIC) - { - /* other signature parameters can't follow a VARIADIC parameter */ + /* other input parameters can't follow a VARIADIC parameter */ if (varCount > 0) ereport(ERROR, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("VARIADIC parameter must be the last signature parameter"))); - sigArgTypes[sigArgCount++] = toid; + errmsg("VARIADIC parameter must be the last input parameter"))); + inTypes[inCount++] = toid; + isinput = true; + if (parameterTypes_list) + *parameterTypes_list = lappend_oid(*parameterTypes_list, toid); } /* handle output parameters */ if (fp->mode != FUNC_PARAM_IN && fp->mode != FUNC_PARAM_VARIADIC) { if (objtype == OBJECT_PROCEDURE) + { + /* + * We disallow OUT-after-VARIADIC only for procedures. While + * such a case causes no confusion in ordinary function calls, + * it would cause confusion in a CALL statement. + */ + if (varCount > 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("VARIADIC parameter must be the last parameter"))); + /* Procedures with output parameters always return RECORD */ *requiredResultType = RECORDOID; + } else if (outCount == 0) /* save first output param's type */ *requiredResultType = toid; outCount++; @@ -432,13 +437,23 @@ interpret_function_parameter_list(ParseState *pstate, ereport(ERROR, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), errmsg("input parameters after one with a default value must also have defaults"))); + + /* + * For procedures, we also can't allow OUT parameters after one + * with a default, because the same sort of confusion arises in a + * CALL statement. + */ + if (objtype == OBJECT_PROCEDURE && have_defaults) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("procedure OUT parameters cannot appear after one with a default value"))); } i++; } /* Now construct the proper outputs as needed */ - *parameterTypes = buildoidvector(sigArgTypes, sigArgCount); + *parameterTypes = buildoidvector(inTypes, inCount); if (outCount > 0 || varCount > 0) { @@ -2179,9 +2194,6 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver int nargs; int i; AclResult aclresult; - Oid *argtypes; - char **argnames; - char *argmodes; FmgrInfo flinfo; CallContext *callcontext; EState *estate; @@ -2224,29 +2236,10 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver if (((Form_pg_proc) GETSTRUCT(tp))->prosecdef) callcontext->atomic = true; - /* - * Expand named arguments, defaults, etc. We do not want to scribble on - * the passed-in CallStmt parse tree, so first flat-copy fexpr, allowing - * us to replace its args field. (Note that expand_function_arguments - * will not modify any of the passed-in data structure.) - */ - { - FuncExpr *nexpr = makeNode(FuncExpr); - - memcpy(nexpr, fexpr, sizeof(FuncExpr)); - fexpr = nexpr; - } - - fexpr->args = expand_function_arguments(fexpr->args, - fexpr->funcresulttype, - tp); - nargs = list_length(fexpr->args); - - get_func_arg_info(tp, &argtypes, &argnames, &argmodes); - ReleaseSysCache(tp); /* safety check; see ExecInitFunc() */ + nargs = list_length(fexpr->args); if (nargs > FUNC_MAX_ARGS) ereport(ERROR, (errcode(ERRCODE_TOO_MANY_ARGUMENTS), @@ -2273,24 +2266,16 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver i = 0; foreach(lc, fexpr->args) { - if (argmodes && argmodes[i] == PROARGMODE_OUT) - { - fcinfo->args[i].value = 0; - fcinfo->args[i].isnull = true; - } - else - { - ExprState *exprstate; - Datum val; - bool isnull; + ExprState *exprstate; + Datum val; + bool isnull; - exprstate = ExecPrepareExpr(lfirst(lc), estate); + exprstate = ExecPrepareExpr(lfirst(lc), estate); - val = ExecEvalExprSwitchContext(exprstate, econtext, &isnull); + val = ExecEvalExprSwitchContext(exprstate, econtext, &isnull); - fcinfo->args[i].value = val; - fcinfo->args[i].isnull = isnull; - } + fcinfo->args[i].value = val; + fcinfo->args[i].isnull = isnull; i++; } diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 39580f7d57..152d3d60ad 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -245,8 +245,7 @@ prepare_sql_fn_parse_info(HeapTuple procedureTuple, if (isNull) proargmodes = PointerGetDatum(NULL); /* just to be sure */ - n_arg_names = get_func_input_arg_names(procedureStruct->prokind, - proargnames, proargmodes, + n_arg_names = get_func_input_arg_names(proargnames, proargmodes, &pinfo->argnames); /* Paranoia: ignore the result if too few array entries */ diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 90770a89b0..3dd467e0e9 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3478,6 +3478,7 @@ _copyCallStmt(const CallStmt *from) COPY_NODE_FIELD(funccall); COPY_NODE_FIELD(funcexpr); + COPY_NODE_FIELD(outargs); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index ce76d093dd..d38cf55e6d 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1241,6 +1241,7 @@ _equalCallStmt(const CallStmt *a, const CallStmt *b) { COMPARE_NODE_FIELD(funccall); COMPARE_NODE_FIELD(funcexpr); + COMPARE_NODE_FIELD(outargs); return true; } diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 517712a8f4..059fa70254 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -124,10 +124,13 @@ static Expr *simplify_function(Oid funcid, Oid result_collid, Oid input_collid, List **args_p, bool funcvariadic, bool process_args, bool allow_non_const, eval_const_expressions_context *context); -static List *reorder_function_arguments(List *args, HeapTuple func_tuple); -static List *add_function_defaults(List *args, HeapTuple func_tuple); +static List *reorder_function_arguments(List *args, int pronargs, + HeapTuple func_tuple); +static List *add_function_defaults(List *args, int pronargs, + HeapTuple func_tuple); static List *fetch_function_defaults(HeapTuple func_tuple); static void recheck_cast_function_args(List *args, Oid result_type, + Oid *proargtypes, int pronargs, HeapTuple func_tuple); static Expr *evaluate_function(Oid funcid, Oid result_type, int32 result_typmod, Oid result_collid, Oid input_collid, List *args, @@ -2326,7 +2329,8 @@ eval_const_expressions_mutator(Node *node, if (!HeapTupleIsValid(func_tuple)) elog(ERROR, "cache lookup failed for function %u", funcid); - args = expand_function_arguments(expr->args, expr->wintype, + args = expand_function_arguments(expr->args, + false, expr->wintype, func_tuple); ReleaseSysCache(func_tuple); @@ -3841,7 +3845,7 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, */ if (process_args) { - args = expand_function_arguments(args, result_type, func_tuple); + args = expand_function_arguments(args, false, result_type, func_tuple); args = (List *) expression_tree_mutator((Node *) args, eval_const_expressions_mutator, (void *) context); @@ -3905,6 +3909,15 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, * expand_function_arguments: convert named-notation args to positional args * and/or insert default args, as needed * + * Returns a possibly-transformed version of the args list. + * + * If include_out_arguments is true, then the args list and the result + * include OUT arguments. + * + * The expected result type of the call must be given, for sanity-checking + * purposes. Also, we ask the caller to provide the function's actual + * pg_proc tuple, not just its OID. + * * If we need to change anything, the input argument list is copied, not * modified. * @@ -3913,12 +3926,46 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, * will fall through very quickly if there's nothing to do. */ List * -expand_function_arguments(List *args, Oid result_type, HeapTuple func_tuple) +expand_function_arguments(List *args, bool include_out_arguments, + Oid result_type, HeapTuple func_tuple) { Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple); + Oid *proargtypes = funcform->proargtypes.values; + int pronargs = funcform->pronargs; bool has_named_args = false; ListCell *lc; + /* + * If we are asked to match to OUT arguments, then use the proallargtypes + * array (which includes those); otherwise use proargtypes (which + * doesn't). Of course, if proallargtypes is null, we always use + * proargtypes. (Fetching proallargtypes is annoyingly expensive + * considering that we may have nothing to do here, but fortunately the + * common case is include_out_arguments == false.) + */ + if (include_out_arguments) + { + Datum proallargtypes; + bool isNull; + + proallargtypes = SysCacheGetAttr(PROCOID, func_tuple, + Anum_pg_proc_proallargtypes, + &isNull); + if (!isNull) + { + ArrayType *arr = DatumGetArrayTypeP(proallargtypes); + + pronargs = ARR_DIMS(arr)[0]; + if (ARR_NDIM(arr) != 1 || + pronargs < 0 || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != OIDOID) + elog(ERROR, "proallargtypes is not a 1-D Oid array or it contains nulls"); + Assert(pronargs >= funcform->pronargs); + proargtypes = (Oid *) ARR_DATA_PTR(arr); + } + } + /* Do we have any named arguments? */ foreach(lc, args) { @@ -3934,16 +3981,20 @@ expand_function_arguments(List *args, Oid result_type, HeapTuple func_tuple) /* If so, we must apply reorder_function_arguments */ if (has_named_args) { - args = reorder_function_arguments(args, func_tuple); + args = reorder_function_arguments(args, pronargs, func_tuple); /* Recheck argument types and add casts if needed */ - recheck_cast_function_args(args, result_type, func_tuple); + recheck_cast_function_args(args, result_type, + proargtypes, pronargs, + func_tuple); } - else if (list_length(args) < funcform->pronargs) + else if (list_length(args) < pronargs) { /* No named args, but we seem to be short some defaults */ - args = add_function_defaults(args, func_tuple); + args = add_function_defaults(args, pronargs, func_tuple); /* Recheck argument types and add casts if needed */ - recheck_cast_function_args(args, result_type, func_tuple); + recheck_cast_function_args(args, result_type, + proargtypes, pronargs, + func_tuple); } return args; @@ -3956,10 +4007,9 @@ expand_function_arguments(List *args, Oid result_type, HeapTuple func_tuple) * impossible to form a truly valid positional call without that. */ static List * -reorder_function_arguments(List *args, HeapTuple func_tuple) +reorder_function_arguments(List *args, int pronargs, HeapTuple func_tuple) { Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple); - int pronargs = funcform->pronargs; int nargsprovided = list_length(args); Node *argarray[FUNC_MAX_ARGS]; ListCell *lc; @@ -3986,6 +4036,7 @@ reorder_function_arguments(List *args, HeapTuple func_tuple) { NamedArgExpr *na = (NamedArgExpr *) arg; + Assert(na->argnumber >= 0 && na->argnumber < pronargs); Assert(argarray[na->argnumber] == NULL); argarray[na->argnumber] = (Node *) na->arg; } @@ -4026,9 +4077,8 @@ reorder_function_arguments(List *args, HeapTuple func_tuple) * and so we know we just need to add defaults at the end. */ static List * -add_function_defaults(List *args, HeapTuple func_tuple) +add_function_defaults(List *args, int pronargs, HeapTuple func_tuple) { - Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple); int nargsprovided = list_length(args); List *defaults; int ndelete; @@ -4037,7 +4087,7 @@ add_function_defaults(List *args, HeapTuple func_tuple) defaults = fetch_function_defaults(func_tuple); /* Delete any unused defaults from the list */ - ndelete = nargsprovided + list_length(defaults) - funcform->pronargs; + ndelete = nargsprovided + list_length(defaults) - pronargs; if (ndelete < 0) elog(ERROR, "not enough default arguments"); if (ndelete > 0) @@ -4086,7 +4136,9 @@ fetch_function_defaults(HeapTuple func_tuple) * caller should have already copied the list structure. */ static void -recheck_cast_function_args(List *args, Oid result_type, HeapTuple func_tuple) +recheck_cast_function_args(List *args, Oid result_type, + Oid *proargtypes, int pronargs, + HeapTuple func_tuple) { Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple); int nargs; @@ -4102,9 +4154,8 @@ recheck_cast_function_args(List *args, Oid result_type, HeapTuple func_tuple) { actual_arg_types[nargs++] = exprType((Node *) lfirst(lc)); } - Assert(nargs == funcform->pronargs); - memcpy(declared_arg_types, funcform->proargtypes.values, - funcform->pronargs * sizeof(Oid)); + Assert(nargs == pronargs); + memcpy(declared_arg_types, proargtypes, pronargs * sizeof(Oid)); rettype = enforce_generic_type_consistency(actual_arg_types, declared_arg_types, nargs, diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 201b88d1ad..23daf99efb 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -25,6 +25,7 @@ #include "postgres.h" #include "access/sysattr.h" +#include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -50,6 +51,7 @@ #include "utils/guc.h" #include "utils/queryjumble.h" #include "utils/rel.h" +#include "utils/syscache.h" /* Hook for plugins to get control at end of parse analysis */ @@ -2933,8 +2935,6 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt) /* * transform a CallStmt - * - * We need to do parse analysis on the procedure call and its arguments. */ static Query * transformCallStmt(ParseState *pstate, CallStmt *stmt) @@ -2942,8 +2942,17 @@ transformCallStmt(ParseState *pstate, CallStmt *stmt) List *targs; ListCell *lc; Node *node; + FuncExpr *fexpr; + HeapTuple proctup; + Datum proargmodes; + bool isNull; + List *outargs = NIL; Query *result; + /* + * First, do standard parse analysis on the procedure call and its + * arguments, allowing us to identify the called procedure. + */ targs = NIL; foreach(lc, stmt->funccall->args) { @@ -2962,8 +2971,85 @@ transformCallStmt(ParseState *pstate, CallStmt *stmt) assign_expr_collations(pstate, node); - stmt->funcexpr = castNode(FuncExpr, node); + fexpr = castNode(FuncExpr, node); + + proctup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fexpr->funcid)); + if (!HeapTupleIsValid(proctup)) + elog(ERROR, "cache lookup failed for function %u", fexpr->funcid); + + /* + * Expand the argument list to deal with named-argument notation and + * default arguments. For ordinary FuncExprs this'd be done during + * planning, but a CallStmt doesn't go through planning, and there seems + * no good reason not to do it here. + */ + fexpr->args = expand_function_arguments(fexpr->args, + true, + fexpr->funcresulttype, + proctup); + + /* Fetch proargmodes; if it's null, there are no output args */ + proargmodes = SysCacheGetAttr(PROCOID, proctup, + Anum_pg_proc_proargmodes, + &isNull); + if (!isNull) + { + /* + * Split the list into input arguments in fexpr->args and output + * arguments in stmt->outargs. INOUT arguments appear in both lists. + */ + ArrayType *arr; + int numargs; + char *argmodes; + List *inargs; + int i; + + arr = DatumGetArrayTypeP(proargmodes); /* ensure not toasted */ + numargs = list_length(fexpr->args); + if (ARR_NDIM(arr) != 1 || + ARR_DIMS(arr)[0] != numargs || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != CHAROID) + elog(ERROR, "proargmodes is not a 1-D char array of length %d or it contains nulls", + numargs); + argmodes = (char *) ARR_DATA_PTR(arr); + + inargs = NIL; + i = 0; + foreach(lc, fexpr->args) + { + Node *n = lfirst(lc); + + switch (argmodes[i]) + { + case PROARGMODE_IN: + case PROARGMODE_VARIADIC: + inargs = lappend(inargs, n); + break; + case PROARGMODE_OUT: + outargs = lappend(outargs, n); + break; + case PROARGMODE_INOUT: + inargs = lappend(inargs, n); + outargs = lappend(outargs, copyObject(n)); + break; + default: + /* note we don't support PROARGMODE_TABLE */ + elog(ERROR, "invalid argmode %c for procedure", + argmodes[i]); + break; + } + i++; + } + fexpr->args = inargs; + } + stmt->funcexpr = fexpr; + stmt->outargs = outargs; + + ReleaseSysCache(proctup); + + /* represent the command as a utility Query */ result = makeNode(Query); result->commandType = CMD_UTILITY; result->utilityStmt = (Node *) stmt; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 9ee90e3f13..2bd09b1575 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -172,7 +172,7 @@ static RoleSpec *makeRoleSpec(RoleSpecType type, int location); static void check_qualified_name(List *names, core_yyscan_t yyscanner); static List *check_func_name(List *names, core_yyscan_t yyscanner); static List *check_indirection(List *indirection, core_yyscan_t yyscanner); -static List *extractArgTypes(ObjectType objtype, List *parameters); +static List *extractArgTypes(List *parameters); static List *extractAggrArgTypes(List *aggrargs); static List *makeOrderedSetArgs(List *directargs, List *orderedargs, core_yyscan_t yyscanner); @@ -385,8 +385,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <accesspriv> privilege %type <list> privileges privilege_list %type <privtarget> privilege_target -%type <objwithargs> function_with_argtypes aggregate_with_argtypes operator_with_argtypes procedure_with_argtypes function_with_argtypes_common -%type <list> function_with_argtypes_list aggregate_with_argtypes_list operator_with_argtypes_list procedure_with_argtypes_list +%type <objwithargs> function_with_argtypes aggregate_with_argtypes operator_with_argtypes +%type <list> function_with_argtypes_list aggregate_with_argtypes_list operator_with_argtypes_list %type <ival> defacl_privilege_target %type <defelt> DefACLOption %type <list> DefACLOptionList @@ -4757,7 +4757,7 @@ AlterExtensionContentsStmt: n->object = (Node *) lcons(makeString($9), $7); $$ = (Node *)n; } - | ALTER EXTENSION name add_drop PROCEDURE procedure_with_argtypes + | ALTER EXTENSION name add_drop PROCEDURE function_with_argtypes { AlterExtensionContentsStmt *n = makeNode(AlterExtensionContentsStmt); n->extname = $3; @@ -4766,7 +4766,7 @@ AlterExtensionContentsStmt: n->object = (Node *) $6; $$ = (Node *)n; } - | ALTER EXTENSION name add_drop ROUTINE procedure_with_argtypes + | ALTER EXTENSION name add_drop ROUTINE function_with_argtypes { AlterExtensionContentsStmt *n = makeNode(AlterExtensionContentsStmt); n->extname = $3; @@ -6505,7 +6505,7 @@ CommentStmt: n->comment = $8; $$ = (Node *) n; } - | COMMENT ON PROCEDURE procedure_with_argtypes IS comment_text + | COMMENT ON PROCEDURE function_with_argtypes IS comment_text { CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_PROCEDURE; @@ -6513,7 +6513,7 @@ CommentStmt: n->comment = $6; $$ = (Node *) n; } - | COMMENT ON ROUTINE procedure_with_argtypes IS comment_text + | COMMENT ON ROUTINE function_with_argtypes IS comment_text { CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_ROUTINE; @@ -6659,7 +6659,7 @@ SecLabelStmt: n->label = $9; $$ = (Node *) n; } - | SECURITY LABEL opt_provider ON PROCEDURE procedure_with_argtypes + | SECURITY LABEL opt_provider ON PROCEDURE function_with_argtypes IS security_label { SecLabelStmt *n = makeNode(SecLabelStmt); @@ -7023,7 +7023,7 @@ privilege_target: n->objs = $2; $$ = n; } - | PROCEDURE procedure_with_argtypes_list + | PROCEDURE function_with_argtypes_list { PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget)); n->targtype = ACL_TARGET_OBJECT; @@ -7031,7 +7031,7 @@ privilege_target: n->objs = $2; $$ = n; } - | ROUTINE procedure_with_argtypes_list + | ROUTINE function_with_argtypes_list { PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget)); n->targtype = ACL_TARGET_OBJECT; @@ -7556,33 +7556,20 @@ function_with_argtypes_list: { $$ = lappend($1, $3); } ; -procedure_with_argtypes_list: - procedure_with_argtypes { $$ = list_make1($1); } - | procedure_with_argtypes_list ',' procedure_with_argtypes - { $$ = lappend($1, $3); } - ; - function_with_argtypes: func_name func_args { ObjectWithArgs *n = makeNode(ObjectWithArgs); n->objname = $1; - n->objargs = extractArgTypes(OBJECT_FUNCTION, $2); + n->objargs = extractArgTypes($2); $$ = n; } - | function_with_argtypes_common - { - $$ = $1; - } - ; - -function_with_argtypes_common: /* * Because of reduce/reduce conflicts, we can't use func_name * below, but we can write it out the long way, which actually * allows more cases. */ - type_func_name_keyword + | type_func_name_keyword { ObjectWithArgs *n = makeNode(ObjectWithArgs); n->objname = list_make1(makeString(pstrdup($1))); @@ -7606,24 +7593,6 @@ function_with_argtypes_common: } ; -/* - * This is different from function_with_argtypes in the call to - * extractArgTypes(). - */ -procedure_with_argtypes: - func_name func_args - { - ObjectWithArgs *n = makeNode(ObjectWithArgs); - n->objname = $1; - n->objargs = extractArgTypes(OBJECT_PROCEDURE, $2); - $$ = n; - } - | function_with_argtypes_common - { - $$ = $1; - } - ; - /* * func_args_with_defaults is separate because we only want to accept * defaults in CREATE FUNCTION, not in ALTER etc. @@ -8052,7 +8021,7 @@ AlterFunctionStmt: n->actions = $4; $$ = (Node *) n; } - | ALTER PROCEDURE procedure_with_argtypes alterfunc_opt_list opt_restrict + | ALTER PROCEDURE function_with_argtypes alterfunc_opt_list opt_restrict { AlterFunctionStmt *n = makeNode(AlterFunctionStmt); n->objtype = OBJECT_PROCEDURE; @@ -8060,7 +8029,7 @@ AlterFunctionStmt: n->actions = $4; $$ = (Node *) n; } - | ALTER ROUTINE procedure_with_argtypes alterfunc_opt_list opt_restrict + | ALTER ROUTINE function_with_argtypes alterfunc_opt_list opt_restrict { AlterFunctionStmt *n = makeNode(AlterFunctionStmt); n->objtype = OBJECT_ROUTINE; @@ -8116,7 +8085,7 @@ RemoveFuncStmt: n->concurrent = false; $$ = (Node *)n; } - | DROP PROCEDURE procedure_with_argtypes_list opt_drop_behavior + | DROP PROCEDURE function_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_PROCEDURE; @@ -8126,7 +8095,7 @@ RemoveFuncStmt: n->concurrent = false; $$ = (Node *)n; } - | DROP PROCEDURE IF_P EXISTS procedure_with_argtypes_list opt_drop_behavior + | DROP PROCEDURE IF_P EXISTS function_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_PROCEDURE; @@ -8136,7 +8105,7 @@ RemoveFuncStmt: n->concurrent = false; $$ = (Node *)n; } - | DROP ROUTINE procedure_with_argtypes_list opt_drop_behavior + | DROP ROUTINE function_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_ROUTINE; @@ -8146,7 +8115,7 @@ RemoveFuncStmt: n->concurrent = false; $$ = (Node *)n; } - | DROP ROUTINE IF_P EXISTS procedure_with_argtypes_list opt_drop_behavior + | DROP ROUTINE IF_P EXISTS function_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_ROUTINE; @@ -8618,7 +8587,7 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name n->missing_ok = true; $$ = (Node *)n; } - | ALTER PROCEDURE procedure_with_argtypes RENAME TO name + | ALTER PROCEDURE function_with_argtypes RENAME TO name { RenameStmt *n = makeNode(RenameStmt); n->renameType = OBJECT_PROCEDURE; @@ -8636,7 +8605,7 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name n->missing_ok = false; $$ = (Node *)n; } - | ALTER ROUTINE procedure_with_argtypes RENAME TO name + | ALTER ROUTINE function_with_argtypes RENAME TO name { RenameStmt *n = makeNode(RenameStmt); n->renameType = OBJECT_ROUTINE; @@ -9047,7 +9016,7 @@ AlterObjectDependsStmt: n->remove = $4; $$ = (Node *)n; } - | ALTER PROCEDURE procedure_with_argtypes opt_no DEPENDS ON EXTENSION name + | ALTER PROCEDURE function_with_argtypes opt_no DEPENDS ON EXTENSION name { AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt); n->objectType = OBJECT_PROCEDURE; @@ -9056,7 +9025,7 @@ AlterObjectDependsStmt: n->remove = $4; $$ = (Node *)n; } - | ALTER ROUTINE procedure_with_argtypes opt_no DEPENDS ON EXTENSION name + | ALTER ROUTINE function_with_argtypes opt_no DEPENDS ON EXTENSION name { AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt); n->objectType = OBJECT_ROUTINE; @@ -9187,7 +9156,7 @@ AlterObjectSchemaStmt: n->missing_ok = false; $$ = (Node *)n; } - | ALTER PROCEDURE procedure_with_argtypes SET SCHEMA name + | ALTER PROCEDURE function_with_argtypes SET SCHEMA name { AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt); n->objectType = OBJECT_PROCEDURE; @@ -9196,7 +9165,7 @@ AlterObjectSchemaStmt: n->missing_ok = false; $$ = (Node *)n; } - | ALTER ROUTINE procedure_with_argtypes SET SCHEMA name + | ALTER ROUTINE function_with_argtypes SET SCHEMA name { AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt); n->objectType = OBJECT_ROUTINE; @@ -9498,7 +9467,7 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec n->newowner = $9; $$ = (Node *)n; } - | ALTER PROCEDURE procedure_with_argtypes OWNER TO RoleSpec + | ALTER PROCEDURE function_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); n->objectType = OBJECT_PROCEDURE; @@ -9506,7 +9475,7 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec n->newowner = $6; $$ = (Node *)n; } - | ALTER ROUTINE procedure_with_argtypes OWNER TO RoleSpec + | ALTER ROUTINE function_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); n->objectType = OBJECT_ROUTINE; @@ -16694,14 +16663,13 @@ check_indirection(List *indirection, core_yyscan_t yyscanner) } /* extractArgTypes() - * * Given a list of FunctionParameter nodes, extract a list of just the - * argument types (TypeNames) for signature parameters only (e.g., only input - * parameters for functions). This is what is needed to look up an existing - * function, which is what is wanted by the productions that use this call. + * argument types (TypeNames) for input parameters only. This is what + * is needed to look up an existing function, which is what is wanted by + * the productions that use this call. */ static List * -extractArgTypes(ObjectType objtype, List *parameters) +extractArgTypes(List *parameters) { List *result = NIL; ListCell *i; @@ -16710,7 +16678,7 @@ extractArgTypes(ObjectType objtype, List *parameters) { FunctionParameter *p = (FunctionParameter *) lfirst(i); - if ((p->mode != FUNC_PARAM_OUT || objtype == OBJECT_PROCEDURE) && p->mode != FUNC_PARAM_TABLE) + if (p->mode != FUNC_PARAM_OUT && p->mode != FUNC_PARAM_TABLE) result = lappend(result, p->argType); } return result; @@ -16723,7 +16691,7 @@ static List * extractAggrArgTypes(List *aggrargs) { Assert(list_length(aggrargs) == 2); - return extractArgTypes(OBJECT_AGGREGATE, (List *) linitial(aggrargs)); + return extractArgTypes((List *) linitial(aggrargs)); } /* makeOrderedSetArgs() diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index baac089d68..eae78fea8f 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -82,7 +82,8 @@ static Oid LookupFuncNameInternal(List *funcname, int nargs, * contain any SRF calls, last_srf can just be pstate->p_last_srf. * * proc_call is true if we are considering a CALL statement, so that the - * name must resolve to a procedure name, not anything else. + * name must resolve to a procedure name, not anything else. This flag + * also specifies that the argument list includes any OUT-mode arguments. */ Node * ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, @@ -264,7 +265,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, fdresult = func_get_detail(funcname, fargs, argnames, nargs, actual_arg_types, - !func_variadic, true, + !func_variadic, true, proc_call, &funcid, &rettype, &retset, &nvargs, &vatype, &declared_arg_types, &argdefaults); @@ -1396,6 +1397,7 @@ func_get_detail(List *funcname, Oid *argtypes, bool expand_variadic, bool expand_defaults, + bool include_out_arguments, Oid *funcid, /* return value */ Oid *rettype, /* return value */ bool *retset, /* return value */ @@ -1420,7 +1422,7 @@ func_get_detail(List *funcname, /* Get list of possible candidates from namespace search */ raw_candidates = FuncnameGetCandidates(funcname, nargs, fargnames, expand_variadic, expand_defaults, - false); + include_out_arguments, false); /* * Quickly check if there is an exact match to the input datatypes (there @@ -1668,7 +1670,7 @@ func_get_detail(List *funcname, defargnumbers = bms_add_member(defargnumbers, firstdefarg[i]); newdefaults = NIL; - i = pform->pronargs - pform->pronargdefaults; + i = best_candidate->nominalnargs - pform->pronargdefaults; foreach(lc, defaults) { if (bms_is_member(i, defargnumbers)) @@ -2057,7 +2059,7 @@ LookupFuncNameInternal(List *funcname, int nargs, const Oid *argtypes, *lookupError = FUNCLOOKUP_NOSUCHFUNC; clist = FuncnameGetCandidates(funcname, nargs, NIL, false, false, - missing_ok); + false, missing_ok); /* * If no arguments were specified, the name must yield a unique candidate. diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c index f998fe2076..e4fb9d31d9 100644 --- a/src/backend/utils/adt/regproc.c +++ b/src/backend/utils/adt/regproc.c @@ -93,7 +93,7 @@ regprocin(PG_FUNCTION_ARGS) * pg_proc entries in the current search path. */ names = stringToQualifiedNameList(pro_name_or_oid); - clist = FuncnameGetCandidates(names, -1, NIL, false, false, false); + clist = FuncnameGetCandidates(names, -1, NIL, false, false, false, false); if (clist == NULL) ereport(ERROR, @@ -127,7 +127,7 @@ to_regproc(PG_FUNCTION_ARGS) * entries in the current search path. */ names = stringToQualifiedNameList(pro_name); - clist = FuncnameGetCandidates(names, -1, NIL, false, false, true); + clist = FuncnameGetCandidates(names, -1, NIL, false, false, false, true); if (clist == NULL || clist->next != NULL) PG_RETURN_NULL(); @@ -175,7 +175,7 @@ regprocout(PG_FUNCTION_ARGS) * qualify it. */ clist = FuncnameGetCandidates(list_make1(makeString(proname)), - -1, NIL, false, false, false); + -1, NIL, false, false, false, false); if (clist != NULL && clist->next == NULL && clist->oid == proid) nspname = NULL; @@ -262,7 +262,8 @@ regprocedurein(PG_FUNCTION_ARGS) */ parseNameAndArgTypes(pro_name_or_oid, false, &names, &nargs, argtypes); - clist = FuncnameGetCandidates(names, nargs, NIL, false, false, false); + clist = FuncnameGetCandidates(names, nargs, NIL, false, false, + false, false); for (; clist; clist = clist->next) { @@ -301,7 +302,7 @@ to_regprocedure(PG_FUNCTION_ARGS) */ parseNameAndArgTypes(pro_name, false, &names, &nargs, argtypes); - clist = FuncnameGetCandidates(names, nargs, NIL, false, false, true); + clist = FuncnameGetCandidates(names, nargs, NIL, false, false, false, true); for (; clist; clist = clist->next) { diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 84ad62caea..fb3d7ab8ef 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -11615,7 +11615,7 @@ generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, if (!force_qualify) p_result = func_get_detail(list_make1(makeString(proname)), NIL, argnames, nargs, argtypes, - !use_variadic, true, + !use_variadic, true, false, &p_funcid, &p_rettype, &p_retset, &p_nvargs, &p_vatype, &p_true_typeids, NULL); diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c index 717b62907c..e94b8037ec 100644 --- a/src/backend/utils/fmgr/funcapi.c +++ b/src/backend/utils/fmgr/funcapi.c @@ -1409,8 +1409,7 @@ get_func_trftypes(HeapTuple procTup, * are set to NULL. You don't get anything if proargnames is NULL. */ int -get_func_input_arg_names(char prokind, - Datum proargnames, Datum proargmodes, +get_func_input_arg_names(Datum proargnames, Datum proargmodes, char ***arg_names) { ArrayType *arr; @@ -1469,7 +1468,6 @@ get_func_input_arg_names(char prokind, if (argmodes == NULL || argmodes[i] == PROARGMODE_IN || argmodes[i] == PROARGMODE_INOUT || - (argmodes[i] == PROARGMODE_OUT && prokind == PROKIND_PROCEDURE) || argmodes[i] == PROARGMODE_VARIADIC) { char *pname = TextDatumGetCString(argnames[i]); diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index aa2774e2d4..b98f284356 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -30,6 +30,7 @@ typedef struct _FuncCandidateList struct _FuncCandidateList *next; int pathpos; /* for internal use of namespace lookup */ Oid oid; /* the function or operator's OID */ + int nominalnargs; /* either pronargs or length(proallargtypes) */ int nargs; /* number of arg types returned */ int nvargs; /* number of args to become variadic array */ int ndargs; /* number of defaulted args */ @@ -99,6 +100,7 @@ extern FuncCandidateList FuncnameGetCandidates(List *names, int nargs, List *argnames, bool expand_variadic, bool expand_defaults, + bool include_out_arguments, bool missing_ok); extern bool FunctionIsVisible(Oid funcid); diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 448d9898cb..a65afe7bc4 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -91,7 +91,7 @@ CATALOG(pg_proc,1255,ProcedureRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(81,Proce * proargtypes */ - /* parameter types (excludes OUT params of functions) */ + /* parameter types (excludes OUT params) */ oidvector proargtypes BKI_LOOKUP(pg_type) BKI_FORCE_NOT_NULL; #ifdef CATALOG_VARLEN diff --git a/src/include/funcapi.h b/src/include/funcapi.h index 83e6bc2a1f..f1304d47e3 100644 --- a/src/include/funcapi.h +++ b/src/include/funcapi.h @@ -172,8 +172,7 @@ extern int get_func_arg_info(HeapTuple procTup, Oid **p_argtypes, char ***p_argnames, char **p_argmodes); -extern int get_func_input_arg_names(char prokind, - Datum proargnames, Datum proargmodes, +extern int get_func_input_arg_names(Datum proargnames, Datum proargmodes, char ***arg_names); extern int get_func_trftypes(HeapTuple procTup, Oid **p_trftypes); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index ef73342019..04789614ce 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2998,13 +2998,19 @@ typedef struct InlineCodeBlock /* ---------------------- * CALL statement + * + * OUT-mode arguments are removed from the transformed funcexpr. The outargs + * list contains copies of the expressions for all output arguments, in the + * order of the procedure's declared arguments. (outargs is never evaluated, + * but is useful to the caller as a reference for what to assign to.) * ---------------------- */ typedef struct CallStmt { NodeTag type; FuncCall *funccall; /* from the parser */ - FuncExpr *funcexpr; /* transformed */ + FuncExpr *funcexpr; /* transformed call, with only input args */ + List *outargs; /* transformed output-argument expressions */ } CallStmt; typedef struct CallContext diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h index 68ebb84bf5..41b49b2662 100644 --- a/src/include/optimizer/optimizer.h +++ b/src/include/optimizer/optimizer.h @@ -153,7 +153,8 @@ extern Node *estimate_expression_value(PlannerInfo *root, Node *node); extern Expr *evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod, Oid result_collation); -extern List *expand_function_arguments(List *args, Oid result_type, +extern List *expand_function_arguments(List *args, bool include_out_arguments, + Oid result_type, struct HeapTupleData *func_tuple); /* in util/predtest.c: */ diff --git a/src/include/parser/parse_func.h b/src/include/parser/parse_func.h index aaf07f8f73..57795a86e4 100644 --- a/src/include/parser/parse_func.h +++ b/src/include/parser/parse_func.h @@ -39,6 +39,7 @@ extern FuncDetailCode func_get_detail(List *funcname, List *fargs, List *fargnames, int nargs, Oid *argtypes, bool expand_variadic, bool expand_defaults, + bool include_out_arguments, Oid *funcid, Oid *rettype, bool *retset, int *nvargs, Oid *vatype, Oid **true_typeids, List **argdefaults); diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out index c804a3ffbf..02887399df 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_call.out +++ b/src/pl/plpgsql/src/expected/plpgsql_call.out @@ -304,6 +304,79 @@ END $$; NOTICE: a: 10, b: <NULL> NOTICE: _a: 10, _b: 20 +CREATE PROCEDURE test_proc10(IN a int, OUT b int, IN c int DEFAULT 11) +LANGUAGE plpgsql +AS $$ +BEGIN + RAISE NOTICE 'a: %, b: %, c: %', a, b, c; + b := a - c; +END; +$$; +DO $$ +DECLARE _a int; _b int; _c int; +BEGIN + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, _b, _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, _b, c => _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(a => _a, b => _b, c => _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, c => _c, b => _b); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, _b); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, b => _b); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(b => _b, a => _a); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; +END +$$; +NOTICE: a: 10, b: <NULL>, c: 7 +NOTICE: _a: 10, _b: 3, _c: 7 +NOTICE: a: 10, b: <NULL>, c: 7 +NOTICE: _a: 10, _b: 3, _c: 7 +NOTICE: a: 10, b: <NULL>, c: 7 +NOTICE: _a: 10, _b: 3, _c: 7 +NOTICE: a: 10, b: <NULL>, c: 7 +NOTICE: _a: 10, _b: 3, _c: 7 +NOTICE: a: 10, b: <NULL>, c: 11 +NOTICE: _a: 10, _b: -1, _c: 7 +NOTICE: a: 10, b: <NULL>, c: 11 +NOTICE: _a: 10, _b: -1, _c: 7 +NOTICE: a: 10, b: <NULL>, c: 11 +NOTICE: _a: 10, _b: -1, _c: 7 +-- OUT + VARIADIC +CREATE PROCEDURE test_proc11(a OUT int, VARIADIC b int[]) +LANGUAGE plpgsql +AS $$ +BEGIN + RAISE NOTICE 'a: %, b: %', a, b; + a := b[1] + b[2]; +END; +$$; +DO $$ +DECLARE _a int; _b int; _c int; +BEGIN + _a := 10; _b := 30; _c := 7; + CALL test_proc11(_a, _b, _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; +END +$$; +NOTICE: a: <NULL>, b: {30,7} +NOTICE: _a: 37, _b: 30, _c: 7 -- transition variable assignment TRUNCATE test1; CREATE FUNCTION triggerfunc1() RETURNS trigger diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index ce8d97447d..a68a2077c3 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -460,7 +460,6 @@ do_compile(FunctionCallInfo fcinfo, /* Remember arguments in appropriate arrays */ if (argmode == PROARGMODE_IN || argmode == PROARGMODE_INOUT || - (argmode == PROARGMODE_OUT && function->fn_prokind == PROKIND_PROCEDURE) || argmode == PROARGMODE_VARIADIC) in_arg_varnos[num_in_args++] = argvariable->dno; if (argmode == PROARGMODE_OUT || diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 78b593d12c..31b9e85744 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -2246,18 +2246,17 @@ make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) { List *plansources; CachedPlanSource *plansource; - Node *node; + CallStmt *stmt; FuncExpr *funcexpr; HeapTuple func_tuple; - List *funcargs; Oid *argtypes; char **argnames; char *argmodes; + int numargs; MemoryContext oldcontext; PLpgSQL_row *row; int nfields; int i; - ListCell *lc; /* Use eval_mcontext for any cruft accumulated here */ oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); @@ -2271,11 +2270,12 @@ make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) plansource = (CachedPlanSource *) linitial(plansources); if (list_length(plansource->query_list) != 1) elog(ERROR, "query for CALL statement is not a CallStmt"); - node = linitial_node(Query, plansource->query_list)->utilityStmt; - if (node == NULL || !IsA(node, CallStmt)) + stmt = (CallStmt *) linitial_node(Query, + plansource->query_list)->utilityStmt; + if (stmt == NULL || !IsA(stmt, CallStmt)) elog(ERROR, "query for CALL statement is not a CallStmt"); - funcexpr = ((CallStmt *) node)->funcexpr; + funcexpr = stmt->funcexpr; func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcexpr->funcid)); @@ -2284,16 +2284,10 @@ make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) funcexpr->funcid); /* - * Extract function arguments, and expand any named-arg notation + * Get the argument names and modes, so that we can deliver on-point error + * messages when something is wrong. */ - 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); + numargs = get_func_arg_info(func_tuple, &argtypes, &argnames, &argmodes); ReleaseSysCache(func_tuple); @@ -2307,7 +2301,7 @@ make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) row->dtype = PLPGSQL_DTYPE_ROW; row->refname = "(unnamed row)"; row->lineno = -1; - row->varnos = (int *) palloc(sizeof(int) * list_length(funcargs)); + row->varnos = (int *) palloc(numargs * sizeof(int)); MemoryContextSwitchTo(get_eval_mcontext(estate)); @@ -2317,15 +2311,14 @@ make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) * Datum. */ nfields = 0; - i = 0; - foreach(lc, funcargs) + for (i = 0; i < numargs; i++) { - Node *n = lfirst(lc); - if (argmodes && (argmodes[i] == PROARGMODE_INOUT || argmodes[i] == PROARGMODE_OUT)) { + Node *n = list_nth(stmt->outargs, nfields); + if (IsA(n, Param)) { Param *param = (Param *) n; @@ -2348,9 +2341,10 @@ make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) i + 1))); } } - i++; } + Assert(nfields == list_length(stmt->outargs)); + row->nfields = nfields; MemoryContextSwitchTo(oldcontext); diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql index c61a75be9b..755935a006 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_call.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql @@ -281,6 +281,68 @@ BEGIN END $$; +CREATE PROCEDURE test_proc10(IN a int, OUT b int, IN c int DEFAULT 11) +LANGUAGE plpgsql +AS $$ +BEGIN + RAISE NOTICE 'a: %, b: %, c: %', a, b, c; + b := a - c; +END; +$$; + +DO $$ +DECLARE _a int; _b int; _c int; +BEGIN + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, _b, _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, _b, c => _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(a => _a, b => _b, c => _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, c => _c, b => _b); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, _b); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(_a, b => _b); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; + + _a := 10; _b := 30; _c := 7; + CALL test_proc10(b => _b, a => _a); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; +END +$$; + +-- OUT + VARIADIC + +CREATE PROCEDURE test_proc11(a OUT int, VARIADIC b int[]) +LANGUAGE plpgsql +AS $$ +BEGIN + RAISE NOTICE 'a: %, b: %', a, b; + a := b[1] + b[2]; +END; +$$; + +DO $$ +DECLARE _a int; _b int; _c int; +BEGIN + _a := 10; _b := 30; _c := 7; + CALL test_proc11(_a, _b, _c); + RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c; +END +$$; + -- transition variable assignment diff --git a/src/pl/plpython/expected/plpython_call.out b/src/pl/plpython/expected/plpython_call.out index c3f3c8e95e..55e1027246 100644 --- a/src/pl/plpython/expected/plpython_call.out +++ b/src/pl/plpython/expected/plpython_call.out @@ -56,7 +56,7 @@ CALL test_proc6(2, 3, 4); CREATE PROCEDURE test_proc9(IN a int, OUT b int) LANGUAGE plpythonu AS $$ -plpy.notice("a: %s, b: %s" % (a, b)) +plpy.notice("a: %s" % (a)) return (a * 2,) $$; DO $$ @@ -67,7 +67,7 @@ BEGIN RAISE NOTICE '_a: %, _b: %', _a, _b; END $$; -NOTICE: a: 10, b: None +NOTICE: a: 10 NOTICE: _a: 10, _b: 20 DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c index b7c0b5cebe..494f109b32 100644 --- a/src/pl/plpython/plpy_procedure.c +++ b/src/pl/plpython/plpy_procedure.c @@ -272,7 +272,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger) /* proc->nargs was initialized to 0 above */ for (i = 0; i < total; i++) { - if ((modes[i] != PROARGMODE_OUT || proc->is_procedure) && + if (modes[i] != PROARGMODE_OUT && modes[i] != PROARGMODE_TABLE) (proc->nargs)++; } @@ -288,7 +288,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger) Form_pg_type argTypeStruct; if (modes && - ((modes[i] == PROARGMODE_OUT && !proc->is_procedure) || + (modes[i] == PROARGMODE_OUT || modes[i] == PROARGMODE_TABLE)) continue; /* skip OUT arguments */ diff --git a/src/pl/plpython/sql/plpython_call.sql b/src/pl/plpython/sql/plpython_call.sql index 46e89b1a9e..b0b3705ae3 100644 --- a/src/pl/plpython/sql/plpython_call.sql +++ b/src/pl/plpython/sql/plpython_call.sql @@ -59,7 +59,7 @@ CALL test_proc6(2, 3, 4); CREATE PROCEDURE test_proc9(IN a int, OUT b int) LANGUAGE plpythonu AS $$ -plpy.notice("a: %s, b: %s" % (a, b)) +plpy.notice("a: %s" % (a)) return (a * 2,) $$; diff --git a/src/pl/tcl/expected/pltcl_call.out b/src/pl/tcl/expected/pltcl_call.out index f0eb356cf2..e4498375ec 100644 --- a/src/pl/tcl/expected/pltcl_call.out +++ b/src/pl/tcl/expected/pltcl_call.out @@ -53,7 +53,7 @@ CALL test_proc6(2, 3, 4); CREATE PROCEDURE test_proc9(IN a int, OUT b int) LANGUAGE pltcl AS $$ -elog NOTICE "a: $1, b: $2" +elog NOTICE "a: $1" return [list b [expr {$1 * 2}]] $$; DO $$ @@ -64,7 +64,7 @@ BEGIN RAISE NOTICE '_a: %, _b: %', _a, _b; END $$; -NOTICE: a: 10, b: +NOTICE: a: 10 NOTICE: _a: 10, _b: 20 DROP PROCEDURE test_proc1; DROP PROCEDURE test_proc2; diff --git a/src/pl/tcl/sql/pltcl_call.sql b/src/pl/tcl/sql/pltcl_call.sql index 963277e1fb..37efbdefc2 100644 --- a/src/pl/tcl/sql/pltcl_call.sql +++ b/src/pl/tcl/sql/pltcl_call.sql @@ -57,7 +57,7 @@ CALL test_proc6(2, 3, 4); CREATE PROCEDURE test_proc9(IN a int, OUT b int) LANGUAGE pltcl AS $$ -elog NOTICE "a: $1, b: $2" +elog NOTICE "a: $1" return [list b [expr {$1 * 2}]] $$; diff --git a/src/test/regress/expected/create_procedure.out b/src/test/regress/expected/create_procedure.out index d45575561e..ec884c4d70 100644 --- a/src/test/regress/expected/create_procedure.out +++ b/src/test/regress/expected/create_procedure.out @@ -217,12 +217,67 @@ AS $$ INSERT INTO cp_test VALUES (1, 'a'); SELECT 1; $$; +-- standard way to do a call: CALL ptest9(NULL); a --- 1 (1 row) +-- you can write an expression, but it's not evaluated +CALL ptest9(1/0); -- no error + a +--- + 1 +(1 row) + +-- ... and it had better match the type of the parameter +CALL ptest9(1./0.); -- error +ERROR: procedure ptest9(numeric) does not exist +LINE 1: CALL ptest9(1./0.); + ^ +HINT: No procedure matches the given name and argument types. You might need to add explicit type casts. +-- check named-parameter matching +CREATE PROCEDURE ptest10(OUT a int, IN b int, IN c int) +LANGUAGE SQL AS $$ SELECT b - c $$; +CALL ptest10(null, 7, 4); + a +--- + 3 +(1 row) + +CALL ptest10(a => null, b => 8, c => 2); + a +--- + 6 +(1 row) + +CALL ptest10(null, 7, c => 2); + a +--- + 5 +(1 row) + +CALL ptest10(null, c => 4, b => 11); + a +--- + 7 +(1 row) + +CALL ptest10(b => 8, c => 2, a => 0); + a +--- + 6 +(1 row) + +CREATE PROCEDURE ptest11(a OUT int, VARIADIC b int[]) LANGUAGE SQL + AS $$ SELECT b[1] + b[2] $$; +CALL ptest11(null, 11, 12, 13); + a +---- + 23 +(1 row) + -- various error cases CALL version(); -- error: not a procedure ERROR: version() is not a procedure @@ -242,6 +297,12 @@ CREATE PROCEDURE ptestx() LANGUAGE SQL STRICT AS $$ INSERT INTO cp_test VALUES ( ERROR: invalid attribute in procedure definition LINE 1: CREATE PROCEDURE ptestx() LANGUAGE SQL STRICT AS $$ INSERT I... ^ +CREATE PROCEDURE ptestx(a VARIADIC int[], b OUT int) LANGUAGE SQL + AS $$ SELECT a[1] $$; +ERROR: VARIADIC parameter must be the last parameter +CREATE PROCEDURE ptestx(a int DEFAULT 42, b OUT int) LANGUAGE SQL + AS $$ SELECT a $$; +ERROR: procedure OUT parameters cannot appear after one with a default value 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 76f781c0b9..b835ab7ec0 100644 --- a/src/test/regress/sql/create_procedure.sql +++ b/src/test/regress/sql/create_procedure.sql @@ -153,7 +153,27 @@ INSERT INTO cp_test VALUES (1, 'a'); SELECT 1; $$; +-- standard way to do a call: CALL ptest9(NULL); +-- you can write an expression, but it's not evaluated +CALL ptest9(1/0); -- no error +-- ... and it had better match the type of the parameter +CALL ptest9(1./0.); -- error + +-- check named-parameter matching +CREATE PROCEDURE ptest10(OUT a int, IN b int, IN c int) +LANGUAGE SQL AS $$ SELECT b - c $$; + +CALL ptest10(null, 7, 4); +CALL ptest10(a => null, b => 8, c => 2); +CALL ptest10(null, 7, c => 2); +CALL ptest10(null, c => 4, b => 11); +CALL ptest10(b => 8, c => 2, a => 0); + +CREATE PROCEDURE ptest11(a OUT int, VARIADIC b int[]) LANGUAGE SQL + AS $$ SELECT b[1] + b[2] $$; + +CALL ptest11(null, 11, 12, 13); -- various error cases @@ -163,6 +183,10 @@ CALL sum(1); -- error: not a procedure CREATE PROCEDURE ptestx() LANGUAGE SQL WINDOW AS $$ INSERT INTO cp_test VALUES (1, 'a') $$; CREATE PROCEDURE ptestx() LANGUAGE SQL STRICT AS $$ INSERT INTO cp_test VALUES (1, 'a') $$; +CREATE PROCEDURE ptestx(a VARIADIC int[], b OUT int) LANGUAGE SQL + AS $$ SELECT a[1] $$; +CREATE PROCEDURE ptestx(a int DEFAULT 42, b OUT int) LANGUAGE SQL + AS $$ SELECT a $$; ALTER PROCEDURE ptest1(text) STRICT; ALTER FUNCTION ptest1(text) VOLATILE; -- error: not a function