Re: CALL versus procedures with output-only arguments - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: CALL versus procedures with output-only arguments |
Date | |
Msg-id | 1032040.1623107400@sss.pgh.pa.us Whole thread Raw |
In response to | Re: CALL versus procedures with output-only arguments (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: CALL versus procedures with output-only arguments
|
List | pgsql-hackers |
I wrote: > Hmm, these are atop HEAD from a week or so back. The cfbot seems to > think they still apply. In any case, I was about to spend some effort > on the docs, so I'll post an updated version soon (hopefully today). Here is said update (rolled up into one patch this time; maybe that will avoid the apply problems you had). I noticed that there is one other loose end in the patch: should LookupFuncName() really be passing OBJECT_ROUTINE to LookupFuncNameInternal()? This matches its old behavior, in which no particular routine type restriction was applied; but I think that the callers are nearly all expecting that only plain functions will be returned. That's more of a possible pre-existing bug than it is the fault of the patch, but nonetheless this might be a good time to resolve it. 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/alter_procedure.sgml b/doc/src/sgml/ref/alter_procedure.sgml index 9cbe2c7cea..033fda92ee 100644 --- a/doc/src/sgml/ref/alter_procedure.sgml +++ b/doc/src/sgml/ref/alter_procedure.sgml @@ -96,7 +96,7 @@ ALTER PROCEDURE <replaceable>name</replaceable> [ ( [ [ <replaceable class="para The name of an argument. Note that <command>ALTER PROCEDURE</command> does not actually pay any attention to argument names, since only the argument data - types are needed to determine the procedure's identity. + types are used to determine the procedure's identity. </para> </listitem> </varlistentry> @@ -108,6 +108,8 @@ ALTER PROCEDURE <replaceable>name</replaceable> [ ( [ [ <replaceable class="para <para> The data type(s) of the procedure's arguments (optionally schema-qualified), if any. + See <xref linkend="sql-dropprocedure"/> for the details of how + the procedure is looked up using the argument data type(s). </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/call.sgml b/doc/src/sgml/ref/call.sgml index abaa81c78b..9e83a77b7c 100644 --- a/doc/src/sgml/ref/call.sgml +++ b/doc/src/sgml/ref/call.sgml @@ -55,9 +55,24 @@ 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. + (Writing something else for an <literal>OUT</literal> parameter + might cause compatibility problems with + future <productname>PostgreSQL</productname> versions.) </para> </listitem> </varlistentry> @@ -101,7 +116,10 @@ CALL do_db_maintenance(); <title>Compatibility</title> <para> - <command>CALL</command> conforms to the SQL standard. + <command>CALL</command> conforms to the SQL standard, + except for the handling of output parameters. The standard + says that users should write variables to receive the values + of output parameters. </para> </refsect1> diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml index 4f30bb93e2..e07fc47fd3 100644 --- a/doc/src/sgml/ref/comment.sgml +++ b/doc/src/sgml/ref/comment.sgml @@ -176,12 +176,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/drop_procedure.sgml b/doc/src/sgml/ref/drop_procedure.sgml index bf2c6ce1aa..af3577dc3b 100644 --- a/doc/src/sgml/ref/drop_procedure.sgml +++ b/doc/src/sgml/ref/drop_procedure.sgml @@ -30,10 +30,10 @@ DROP PROCEDURE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ <title>Description</title> <para> - <command>DROP PROCEDURE</command> removes the definition of an existing - procedure. To execute this command the user must be the - owner of the procedure. The argument types to the - procedure must be specified, since several different procedures + <command>DROP PROCEDURE</command> removes the definition of one or more + existing procedures. To execute this command the user must be the + owner of the procedure(s). The argument types to the + procedure(s) usually must be specified, since several different procedures can exist with the same name and different argument lists. </para> </refsect1> @@ -56,8 +56,7 @@ DROP PROCEDURE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ <term><replaceable class="parameter">name</replaceable></term> <listitem> <para> - The name (optionally schema-qualified) of an existing procedure. If no - argument list is specified, the name must be unique in its schema. + The name (optionally schema-qualified) of an existing procedure. </para> </listitem> </varlistentry> @@ -69,7 +68,7 @@ DROP PROCEDURE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ <para> The mode of an argument: <literal>IN</literal>, <literal>OUT</literal>, <literal>INOUT</literal>, or <literal>VARIADIC</literal>. If omitted, - the default is <literal>IN</literal>. + the default is <literal>IN</literal> (but see below). </para> </listitem> </varlistentry> @@ -82,7 +81,7 @@ DROP PROCEDURE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ The name of an argument. Note that <command>DROP PROCEDURE</command> does not actually pay any attention to argument names, since only the argument data - types are needed to determine the procedure's identity. + types are used to determine the procedure's identity. </para> </listitem> </varlistentry> @@ -94,6 +93,7 @@ DROP PROCEDURE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ <para> The data type(s) of the procedure's arguments (optionally schema-qualified), if any. + See below for details. </para> </listitem> </varlistentry> @@ -121,12 +121,81 @@ DROP PROCEDURE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ </variablelist> </refsect1> + <refsect1 id="sql-dropprocedure-notes"> + <title>Notes</title> + + <para> + If there is only one procedure by the given name, the argument list + can be omitted. Omit the parentheses too in this case. + </para> + + <para> + In <productname>PostgreSQL</productname>, it's sufficient to list the + data types of input (including <literal>INOUT</literal>) arguments, + because no two routines of the same name are allowed to share the same + input-argument list. Moreover, the <command>DROP</command> command + will not actually check that you wrote the types + of <literal>OUT</literal> arguments correctly; so any arguments that + are explicitly marked <literal>OUT</literal> are just noise. But + writing them is recommendable for consistency with the + corresponding <command>CREATE</command> command. + </para> + + <para> + For compatibility with the SQL standard, it is also allowed to write + all the argument data types (including those of <literal>OUT</literal> + arguments) without + any <replaceable class="parameter">argmode</replaceable> markers. + When this is done, the types of the procedure's <literal>OUT</literal> + argument(s) <emphasis>will</emphasis> be verified against the command. + This provision creates an ambiguity, in that when the argument list + contains no <replaceable class="parameter">argmode</replaceable> + markers, it's unclear which rule is intended. + The <command>DROP</command> command will attempt the lookup both ways, + and will throw an error if two different procedures are found. + To avoid the risk of such ambiguity, it's recommendable to + write <literal>IN</literal> markers explicitly rather than letting them + be defaulted, thus forcing the + traditional <productname>PostgreSQL</productname> interpretation to be + used. + </para> + + <para> + The lookup rules just explained are also used by other commands that + act on existing procedures, such as <command>ALTER PROCEDURE</command> + and <command>COMMENT ON PROCEDURE</command>. + </para> + </refsect1> + <refsect1 id="sql-dropprocedure-examples"> <title>Examples</title> + <para> + If there is only one procedure <literal>do_db_maintenance</literal>, + this command is sufficient to drop it: +<programlisting> +DROP PROCEDURE do_db_maintenance; +</programlisting> + </para> + + <para> + Given this procedure definition: <programlisting> -DROP PROCEDURE do_db_maintenance(); +CREATE PROCEDURE do_db_maintenance(IN target_schema text, OUT results text) ... </programlisting> + any one of these commands would work to drop it: +<programlisting> +DROP PROCEDURE do_db_maintenance(IN target_schema text, OUT results text); +DROP PROCEDURE do_db_maintenance(IN text, OUT text); +DROP PROCEDURE do_db_maintenance(IN text); +DROP PROCEDURE do_db_maintenance(text); +DROP PROCEDURE do_db_maintenance(text, text); -- potentially ambiguous +</programlisting> + However, the last example would be ambiguous if there is also, say, +<programlisting> +CREATE PROCEDURE do_db_maintenance(IN target_schema text, IN options text) ... +</programlisting> + </para> </refsect1> <refsect1 id="sql-dropprocedure-compatibility"> @@ -140,12 +209,14 @@ DROP PROCEDURE do_db_maintenance(); <para>The standard only allows one procedure to be dropped per command.</para> </listitem> <listitem> - <para>The <literal>IF EXISTS</literal> option</para> + <para>The <literal>IF EXISTS</literal> option is an extension.</para> </listitem> <listitem> - <para>The ability to specify argument modes and names</para> + <para>The ability to specify argument modes and names is an + extension, and the lookup rules differ when modes are given.</para> </listitem> - </itemizedlist></para> + </itemizedlist> + </para> </refsect1> <refsect1> diff --git a/doc/src/sgml/ref/drop_routine.sgml b/doc/src/sgml/ref/drop_routine.sgml index 6c50eb44a1..869884a2cc 100644 --- a/doc/src/sgml/ref/drop_routine.sgml +++ b/doc/src/sgml/ref/drop_routine.sgml @@ -30,15 +30,44 @@ DROP ROUTINE [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [ ( <title>Description</title> <para> - <command>DROP ROUTINE</command> removes the definition of an existing - routine, which can be an aggregate function, a normal function, or a - procedure. See + <command>DROP ROUTINE</command> removes the definition of one or more + existing routines. The term <quote>routine</quote> includes + aggregate functions, normal functions, and procedures. See under <xref linkend="sql-dropaggregate"/>, <xref linkend="sql-dropfunction"/>, and <xref linkend="sql-dropprocedure"/> for the description of the parameters, more examples, and further details. </para> </refsect1> + <refsect1 id="sql-droproutine-notes"> + <title>Notes</title> + + <para> + The lookup rules used by <command>DROP ROUTINE</command> are + fundamentally the same as for <command>DROP PROCEDURE</command>; in + particular, <command>DROP ROUTINE</command> shares that command's + behavior of considering an argument list that has + no <replaceable class="parameter">argmode</replaceable> markers to be + possibly using the SQL standard's definition that <literal>OUT</literal> + arguments are included in the list. (<command>DROP AGGREGATE</command> + and <command>DROP FUNCTION</command> do not do that.) + </para> + + <para> + In some cases where the same name is shared by routines of different + kinds, it is possible for <command>DROP ROUTINE</command> to fail with + an ambiguity error when a more specific command (<command>DROP + FUNCTION</command>, etc.) would work. Specifying the argument type + list more carefully will also resolve such problems. + </para> + + <para> + These lookup rules are also used by other commands that + act on existing routines, such as <command>ALTER ROUTINE</command> + and <command>COMMENT ON ROUTINE</command>. + </para> + </refsect1> + <refsect1 id="sql-droproutine-examples"> <title>Examples</title> @@ -64,15 +93,17 @@ DROP ROUTINE foo(integer); <para>The standard only allows one routine to be dropped per command.</para> </listitem> <listitem> - <para>The <literal>IF EXISTS</literal> option</para> + <para>The <literal>IF EXISTS</literal> option is an extension.</para> </listitem> <listitem> - <para>The ability to specify argument modes and names</para> + <para>The ability to specify argument modes and names is an + extension, and the lookup rules differ when modes are given.</para> </listitem> <listitem> - <para>Aggregate functions are an extension.</para> + <para>User-definable aggregate functions are an extension.</para> </listitem> - </itemizedlist></para> + </itemizedlist> + </para> </refsect1> <refsect1> diff --git a/doc/src/sgml/ref/security_label.sgml b/doc/src/sgml/ref/security_label.sgml index 407a09720b..20a839ff0c 100644 --- a/doc/src/sgml/ref/security_label.sgml +++ b/doc/src/sgml/ref/security_label.sgml @@ -126,12 +126,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..736d04780a 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)); @@ -227,11 +227,16 @@ interpret_function_parameter_list(ParseState *pstate, { FunctionParameter *fp = (FunctionParameter *) lfirst(x); TypeName *t = fp->argType; + FunctionParameterMode fpmode = fp->mode; bool isinput = false; Oid toid; Type typtup; AclResult aclresult; + /* For our purposes here, a defaulted mode spec is identical to IN */ + if (fpmode == FUNC_PARAM_DEFAULT) + fpmode = FUNC_PARAM_IN; + typtup = LookupTypeName(NULL, t, NULL, false); if (typtup) { @@ -288,37 +293,42 @@ interpret_function_parameter_list(ParseState *pstate, } /* handle input parameters */ - if (fp->mode != FUNC_PARAM_OUT && fp->mode != FUNC_PARAM_TABLE) + if (fpmode != FUNC_PARAM_OUT && fpmode != 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 (fpmode != FUNC_PARAM_IN && fpmode != 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++; } - if (fp->mode == FUNC_PARAM_VARIADIC) + if (fpmode == FUNC_PARAM_VARIADIC) { *variadicArgType = toid; varCount++; @@ -341,7 +351,7 @@ interpret_function_parameter_list(ParseState *pstate, allTypes[i] = ObjectIdGetDatum(toid); - paramModes[i] = CharGetDatum(fp->mode); + paramModes[i] = CharGetDatum(fpmode); if (fp->name && fp->name[0]) { @@ -356,19 +366,24 @@ interpret_function_parameter_list(ParseState *pstate, foreach(px, parameters) { FunctionParameter *prevfp = (FunctionParameter *) lfirst(px); + FunctionParameterMode prevfpmode; if (prevfp == fp) break; + /* as above, default mode is IN */ + prevfpmode = prevfp->mode; + if (prevfpmode == FUNC_PARAM_DEFAULT) + prevfpmode = FUNC_PARAM_IN; /* pure in doesn't conflict with pure out */ - if ((fp->mode == FUNC_PARAM_IN || - fp->mode == FUNC_PARAM_VARIADIC) && - (prevfp->mode == FUNC_PARAM_OUT || - prevfp->mode == FUNC_PARAM_TABLE)) + if ((fpmode == FUNC_PARAM_IN || + fpmode == FUNC_PARAM_VARIADIC) && + (prevfpmode == FUNC_PARAM_OUT || + prevfpmode == FUNC_PARAM_TABLE)) continue; - if ((prevfp->mode == FUNC_PARAM_IN || - prevfp->mode == FUNC_PARAM_VARIADIC) && - (fp->mode == FUNC_PARAM_OUT || - fp->mode == FUNC_PARAM_TABLE)) + if ((prevfpmode == FUNC_PARAM_IN || + prevfpmode == FUNC_PARAM_VARIADIC) && + (fpmode == FUNC_PARAM_OUT || + fpmode == FUNC_PARAM_TABLE)) continue; if (prevfp->name && prevfp->name[0] && strcmp(prevfp->name, fp->name) == 0) @@ -432,13 +447,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 +2204,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 +2246,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 +2276,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..32668f85a1 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 */ @@ -1536,7 +1535,7 @@ check_sql_fn_statements(List *queryTreeLists) Query *query = lfirst_node(Query, lc2); /* - * Disallow procedures with output arguments. The current + * Disallow calling procedures with output arguments. 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 @@ -1545,31 +1544,12 @@ check_sql_fn_statements(List *queryTreeLists) 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 arguments is not supported in SQL functions"))); - } + CallStmt *stmt = (CallStmt *) query->utilityStmt; + + if (stmt->outargs != NIL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("calling procedures with output arguments is not supported in SQL functions"))); } } } diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 621f7ce068..bd87f23784 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3407,6 +3407,7 @@ _copyObjectWithArgs(const ObjectWithArgs *from) COPY_NODE_FIELD(objname); COPY_NODE_FIELD(objargs); + COPY_NODE_FIELD(objfuncargs); COPY_SCALAR_FIELD(args_unspecified); return newnode; @@ -3478,6 +3479,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 3033c1934c..dba3e6b31e 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1182,6 +1182,7 @@ _equalObjectWithArgs(const ObjectWithArgs *a, const ObjectWithArgs *b) { COMPARE_NODE_FIELD(objname); COMPARE_NODE_FIELD(objargs); + COMPARE_NODE_FIELD(objfuncargs); COMPARE_SCALAR_FIELD(args_unspecified); return true; @@ -1241,6 +1242,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 9cede29d6a..438b077004 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..284f6b1978 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,21 @@ 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->objfuncargs = $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 +7594,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. @@ -7673,7 +7643,7 @@ func_arg: FunctionParameter *n = makeNode(FunctionParameter); n->name = $1; n->argType = $2; - n->mode = FUNC_PARAM_IN; + n->mode = FUNC_PARAM_DEFAULT; n->defexpr = NULL; $$ = n; } @@ -7691,7 +7661,7 @@ func_arg: FunctionParameter *n = makeNode(FunctionParameter); n->name = NULL; n->argType = $1; - n->mode = FUNC_PARAM_IN; + n->mode = FUNC_PARAM_DEFAULT; n->defexpr = NULL; $$ = n; } @@ -7763,7 +7733,8 @@ func_arg_with_default: /* Aggregate args can be most things that function args can be */ aggr_arg: func_arg { - if (!($1->mode == FUNC_PARAM_IN || + if (!($1->mode == FUNC_PARAM_DEFAULT || + $1->mode == FUNC_PARAM_IN || $1->mode == FUNC_PARAM_VARIADIC)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -7832,6 +7803,7 @@ aggregate_with_argtypes: ObjectWithArgs *n = makeNode(ObjectWithArgs); n->objname = $1; n->objargs = extractAggrArgTypes($2); + n->objfuncargs = (List *) linitial($2); $$ = n; } ; @@ -8052,7 +8024,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 +8032,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 +8088,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 +8098,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 +8108,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 +8118,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 +8590,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 +8608,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 +9019,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 +9028,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 +9159,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 +9168,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 +9470,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 +9478,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 +16666,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 +16681,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 +16694,7 @@ static List * extractAggrArgTypes(List *aggrargs) { Assert(list_length(aggrargs) == 2); - return extractArgTypes(OBJECT_AGGREGATE, (List *) linitial(aggrargs)); + return extractArgTypes((List *) linitial(aggrargs)); } /* makeOrderedSetArgs() @@ -17019,7 +16990,9 @@ mergeTableFuncParameters(List *func_args, List *columns) { FunctionParameter *p = (FunctionParameter *) lfirst(lc); - if (p->mode != FUNC_PARAM_IN && p->mode != FUNC_PARAM_VARIADIC) + if (p->mode != FUNC_PARAM_DEFAULT && + p->mode != FUNC_PARAM_IN && + p->mode != FUNC_PARAM_VARIADIC) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("OUT and INOUT arguments aren't allowed in TABLE functions"))); diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index fb0ba58ff7..262e5ab6b4 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -50,7 +50,9 @@ static Node *ParseComplexProjection(ParseState *pstate, const char *funcname, Node *first_arg, int location); static Oid LookupFuncNameInternal(List *funcname, int nargs, const Oid *argtypes, - bool missing_ok, FuncLookupError *lookupError); + ObjectType objtype, + bool include_out_arguments, bool missing_ok, + FuncLookupError *lookupError); /* @@ -82,7 +84,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, @@ -263,7 +266,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); @@ -1395,6 +1398,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 */ @@ -1419,7 +1423,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 @@ -1667,7 +1671,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)) @@ -2041,12 +2045,15 @@ func_signature_string(List *funcname, int nargs, * * Possible errors: * FUNCLOOKUP_NOSUCHFUNC: we can't find a function of this name. - * FUNCLOOKUP_AMBIGUOUS: nargs == -1 and more than one function matches. + * FUNCLOOKUP_AMBIGUOUS: more than one function matches. */ static Oid LookupFuncNameInternal(List *funcname, int nargs, const Oid *argtypes, - bool missing_ok, FuncLookupError *lookupError) + ObjectType objtype, + bool include_out_arguments, bool missing_ok, + FuncLookupError *lookupError) { + Oid result = InvalidOid; FuncCandidateList clist; /* NULL argtypes allowed for nullary functions only */ @@ -2055,43 +2062,62 @@ LookupFuncNameInternal(List *funcname, int nargs, const Oid *argtypes, /* Always set *lookupError, to forestall uninitialized-variable warnings */ *lookupError = FUNCLOOKUP_NOSUCHFUNC; + /* Get list of candidate objects */ clist = FuncnameGetCandidates(funcname, nargs, NIL, false, false, - missing_ok); + include_out_arguments, missing_ok); - /* - * If no arguments were specified, the name must yield a unique candidate. - */ - if (nargs < 0) + /* Scan list for a match to the arg types (if specified) and the objtype */ + for (; clist != NULL; clist = clist->next) { - if (clist) + /* Check arg type match, if specified */ + if (nargs >= 0) { - /* If there is a second match then it's ambiguous */ - if (clist->next) - { - *lookupError = FUNCLOOKUP_AMBIGUOUS; - return InvalidOid; - } - /* Otherwise return the match */ - return clist->oid; + /* if nargs==0, argtypes can be null; don't pass that to memcmp */ + if (nargs > 0 && + memcmp(argtypes, clist->args, nargs * sizeof(Oid)) != 0) + continue; } - else + + /* Check for duplicates reported by FuncnameGetCandidates */ + if (!OidIsValid(clist->oid)) + { + *lookupError = FUNCLOOKUP_AMBIGUOUS; return InvalidOid; - } + } - /* - * Otherwise, look for a match to the arg types. FuncnameGetCandidates - * has ensured that there's at most one match in the returned list. - */ - while (clist) - { - /* if nargs==0, argtypes can be null; don't pass that to memcmp */ - if (nargs == 0 || - memcmp(argtypes, clist->args, nargs * sizeof(Oid)) == 0) - return clist->oid; - clist = clist->next; + /* Check objtype match, if specified */ + switch (objtype) + { + case OBJECT_FUNCTION: + case OBJECT_AGGREGATE: + /* Ignore procedures */ + if (get_func_prokind(clist->oid) == PROKIND_PROCEDURE) + continue; + break; + case OBJECT_PROCEDURE: + /* Ignore non-procedures */ + if (get_func_prokind(clist->oid) != PROKIND_PROCEDURE) + continue; + break; + case OBJECT_ROUTINE: + /* no restriction */ + break; + default: + Assert(false); + } + + /* Check for multiple matches */ + if (OidIsValid(result)) + { + *lookupError = FUNCLOOKUP_AMBIGUOUS; + return InvalidOid; + } + + /* OK, we have a candidate */ + result = clist->oid; } - return InvalidOid; + return result; } /* @@ -2118,7 +2144,9 @@ LookupFuncName(List *funcname, int nargs, const Oid *argtypes, bool missing_ok) Oid funcoid; FuncLookupError lookupError; - funcoid = LookupFuncNameInternal(funcname, nargs, argtypes, missing_ok, + funcoid = LookupFuncNameInternal(funcname, nargs, argtypes, + OBJECT_ROUTINE, /* XXX okay? */ + false, missing_ok, &lookupError); if (OidIsValid(funcoid)) @@ -2207,10 +2235,14 @@ LookupFuncWithArgs(ObjectType objtype, ObjectWithArgs *func, bool missing_ok) FUNC_MAX_ARGS))); } + /* + * First, perform a lookup considering only input arguments (traditional + * Postgres rules). + */ i = 0; foreach(args_item, func->objargs) { - TypeName *t = (TypeName *) lfirst(args_item); + TypeName *t = lfirst_node(TypeName, args_item); argoids[i] = LookupTypeNameOid(NULL, t, missing_ok); if (!OidIsValid(argoids[i])) @@ -2224,9 +2256,82 @@ LookupFuncWithArgs(ObjectType objtype, ObjectWithArgs *func, bool missing_ok) */ nargs = func->args_unspecified ? -1 : argcount; - oid = LookupFuncNameInternal(func->objname, nargs, argoids, missing_ok, + /* + * In args_unspecified mode, also tell LookupFuncNameInternal to consider + * the object type, since there seems no reason not to. However, if we + * have an argument list, disable the objtype check, because we'd rather + * complain about "object is of wrong type" than "object doesn't exist". + * (Note that with args, FuncnameGetCandidates will have ensured there's + * only one argtype match, so we're not risking an ambiguity failure via + * this choice.) + */ + oid = LookupFuncNameInternal(func->objname, nargs, argoids, + func->args_unspecified ? objtype : OBJECT_ROUTINE, + false, missing_ok, &lookupError); + /* + * If PROCEDURE or ROUTINE was specified, and we have an argument list + * that contains no parameter mode markers, and we didn't already discover + * that there's ambiguity, perform a lookup considering all arguments. + * (Note: for a zero-argument procedure, or in args_unspecified mode, the + * normal lookup is sufficient; so it's OK to require non-NIL objfuncargs + * to perform this lookup.) + */ + if ((objtype == OBJECT_PROCEDURE || objtype == OBJECT_ROUTINE) && + func->objfuncargs != NIL && + lookupError != FUNCLOOKUP_AMBIGUOUS) + { + bool have_param_mode = false; + + /* + * Check for non-default parameter mode markers. If there are any, + * then the command does not conform to SQL-spec syntax, so we may + * assume that the traditional Postgres lookup method of considering + * only input parameters is sufficient. (Note that because the spec + * doesn't have OUT arguments for functions, we also don't need this + * hack in FUNCTION or AGGREGATE mode.) + */ + foreach(args_item, func->objfuncargs) + { + FunctionParameter *fp = lfirst_node(FunctionParameter, args_item); + + if (fp->mode != FUNC_PARAM_DEFAULT) + { + have_param_mode = true; + break; + } + } + + if (!have_param_mode) + { + Oid poid; + + /* Without mode marks, objargs surely includes all params */ + Assert(list_length(func->objfuncargs) == argcount); + + /* For objtype == OBJECT_PROCEDURE, we can ignore non-procedures */ + poid = LookupFuncNameInternal(func->objname, argcount, argoids, + objtype, true, missing_ok, + &lookupError); + + /* Combine results, handling ambiguity */ + if (OidIsValid(poid)) + { + if (OidIsValid(oid) && oid != poid) + { + /* oops, we got hits both ways, on different objects */ + oid = InvalidOid; + lookupError = FUNCLOOKUP_AMBIGUOUS; + } + else + oid = poid; + } + else if (lookupError == FUNCLOOKUP_AMBIGUOUS) + oid = InvalidOid; + } + } + if (OidIsValid(oid)) { /* @@ -2235,6 +2340,10 @@ LookupFuncWithArgs(ObjectType objtype, ObjectWithArgs *func, bool missing_ok) * we allow the objtype of FUNCTION to include aggregates and window * functions; but we draw the line if the object is a procedure. That * is a new enough feature that this historical rule does not apply. + * + * (This check is partially redundant with the objtype check in + * LookupFuncNameInternal; but not entirely, since we often don't tell + * LookupFuncNameInternal to apply that check at all.) */ switch (objtype) { @@ -2345,28 +2454,32 @@ LookupFuncWithArgs(ObjectType objtype, ObjectWithArgs *func, bool missing_ok) (errcode(ERRCODE_AMBIGUOUS_FUNCTION), errmsg("function name \"%s\" is not unique", NameListToString(func->objname)), - errhint("Specify the argument list to select the function unambiguously."))); + func->args_unspecified ? + errhint("Specify the argument list to select the function unambiguously.") : 0)); break; case OBJECT_PROCEDURE: ereport(ERROR, (errcode(ERRCODE_AMBIGUOUS_FUNCTION), errmsg("procedure name \"%s\" is not unique", NameListToString(func->objname)), - errhint("Specify the argument list to select the procedure unambiguously."))); + func->args_unspecified ? + errhint("Specify the argument list to select the procedure unambiguously.") : 0)); break; case OBJECT_AGGREGATE: ereport(ERROR, (errcode(ERRCODE_AMBIGUOUS_FUNCTION), errmsg("aggregate name \"%s\" is not unique", NameListToString(func->objname)), - errhint("Specify the argument list to select the aggregate unambiguously."))); + func->args_unspecified ? + errhint("Specify the argument list to select the aggregate unambiguously.") : 0)); break; case OBJECT_ROUTINE: ereport(ERROR, (errcode(ERRCODE_AMBIGUOUS_FUNCTION), errmsg("routine name \"%s\" is not unique", NameListToString(func->objname)), - errhint("Specify the argument list to select the routine unambiguously."))); + func->args_unspecified ? + errhint("Specify the argument list to select the routine unambiguously.") : 0)); break; default: diff --git a/src/backend/parser/parse_oper.c b/src/backend/parser/parse_oper.c index c379d72fce..4e46079990 100644 --- a/src/backend/parser/parse_oper.c +++ b/src/backend/parser/parse_oper.c @@ -150,8 +150,8 @@ LookupOperWithArgs(ObjectWithArgs *oper, bool noError) rightoid; Assert(list_length(oper->objargs) == 2); - oprleft = linitial(oper->objargs); - oprright = lsecond(oper->objargs); + oprleft = linitial_node(TypeName, oper->objargs); + oprright = lsecond_node(TypeName, oper->objargs); if (oprleft == NULL) leftoid = InvalidOid; 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..3719755a0d 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -3217,7 +3217,15 @@ print_function_arguments(StringInfo buf, HeapTuple proctup, switch (argmode) { case PROARGMODE_IN: - modename = ""; + + /* + * For procedures, explicitly mark all argument modes, so as + * to avoid ambiguity with the SQL syntax for DROP PROCEDURE. + */ + if (proc->prokind == PROKIND_PROCEDURE) + modename = "IN "; + else + modename = ""; isinput = true; break; case PROARGMODE_INOUT: @@ -11615,7 +11623,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/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 4bc845e57c..244507c97c 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1898,7 +1898,7 @@ my %tests = ( create_sql => 'CREATE PROCEDURE dump_test.ptest1(a int) LANGUAGE SQL AS $$ INSERT INTO dump_test.test_table (col1) VALUES (a) $$;', regexp => qr/^ - \QCREATE PROCEDURE dump_test.ptest1(a integer)\E + \QCREATE PROCEDURE dump_test.ptest1(IN a integer)\E \n\s+\QLANGUAGE sql\E \n\s+AS\ \$\$\Q INSERT INTO dump_test.test_table (col1) VALUES (a) \E\$\$; /xm, 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..58328c4377 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2024,18 +2024,29 @@ typedef struct GrantStmt } GrantStmt; /* - * Note: ObjectWithArgs carries only the types of the input parameters of the - * function. So it is sufficient to identify an existing function, but it - * is not enough info to define a function nor to call it. + * ObjectWithArgs represents a function/procedure/operator name plus parameter + * identification. + * + * objargs includes only the types of the input parameters of the object. + * In some contexts, that will be all we have, and it's enough to look up + * objects according to the traditional Postgres rules (i.e., when only input + * arguments matter). + * + * objfuncargs, if not NIL, carries the full specification of the parameter + * list, including parameter mode annotations. + * + * Some grammar productions can set args_unspecified = true instead of + * providing parameter info. In this case, lookup will succeed only if + * the object name is unique. Note that otherwise, NIL parameter lists + * mean zero arguments. */ typedef struct ObjectWithArgs { NodeTag type; List *objname; /* qualified name of function/operator */ - List *objargs; /* list of Typename nodes */ - bool args_unspecified; /* argument list was omitted, so name must - * be unique (note that objargs == NIL - * means zero args) */ + List *objargs; /* list of Typename nodes (input args only) */ + List *objfuncargs; /* list of FunctionParameter nodes */ + bool args_unspecified; /* argument list was omitted? */ } ObjectWithArgs; /* @@ -2955,7 +2966,9 @@ typedef enum FunctionParameterMode FUNC_PARAM_OUT = 'o', /* output only */ FUNC_PARAM_INOUT = 'b', /* both */ FUNC_PARAM_VARIADIC = 'v', /* variadic (always input) */ - FUNC_PARAM_TABLE = 't' /* table function output column */ + FUNC_PARAM_TABLE = 't', /* table function output column */ + /* this is not used in pg_proc: */ + FUNC_PARAM_DEFAULT = 'd' /* default; effectively same as IN */ } FunctionParameterMode; typedef struct FunctionParameter @@ -2998,13 +3011,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..46c827f979 100644 --- a/src/test/regress/expected/create_procedure.out +++ b/src/test/regress/expected/create_procedure.out @@ -19,17 +19,17 @@ $$; List of functions Schema | Name | Result data type | Argument data types | Type --------+--------+------------------+---------------------+------ - public | ptest1 | | x text | proc + public | ptest1 | | IN x text | proc (1 row) SELECT pg_get_functiondef('ptest1'::regproc); - pg_get_functiondef ---------------------------------------------------- - CREATE OR REPLACE PROCEDURE public.ptest1(x text)+ - LANGUAGE sql + - AS $procedure$ + - INSERT INTO cp_test VALUES (1, x); + - $procedure$ + + pg_get_functiondef +------------------------------------------------------ + CREATE OR REPLACE PROCEDURE public.ptest1(IN x text)+ + LANGUAGE sql + + AS $procedure$ + + INSERT INTO cp_test VALUES (1, x); + + $procedure$ + (1 row) @@ -46,7 +46,7 @@ SELECT pg_get_functiondef('ptest1'::regproc); List of functions Schema | Name | Result data type | Argument data types | Type --------+--------+------------------+---------------------+------ - public | ptest1 | | x text | proc + public | ptest1 | | IN x text | proc (1 row) SELECT ptest1('x'); -- error @@ -75,18 +75,18 @@ END; List of functions Schema | Name | Result data type | Argument data types | Type --------+---------+------------------+---------------------+------ - public | ptest1s | | x text | proc + public | ptest1s | | IN x text | proc (1 row) SELECT pg_get_functiondef('ptest1s'::regproc); - pg_get_functiondef ----------------------------------------------------- - CREATE OR REPLACE PROCEDURE public.ptest1s(x text)+ - LANGUAGE sql + - BEGIN ATOMIC + - INSERT INTO cp_test (a, b) + - VALUES (1, ptest1s.x); + - END + + pg_get_functiondef +------------------------------------------------------- + CREATE OR REPLACE PROCEDURE public.ptest1s(IN x text)+ + LANGUAGE sql + + BEGIN ATOMIC + + INSERT INTO cp_test (a, b) + + VALUES (1, ptest1s.x); + + END + (1 row) @@ -196,16 +196,16 @@ END; List of functions Schema | Name | Result data type | Argument data types | Type --------+--------+------------------+---------------------+------ - public | ptest8 | | x text | proc + public | ptest8 | | IN x text | proc (1 row) SELECT pg_get_functiondef('ptest8'::regproc); - pg_get_functiondef ---------------------------------------------------- - CREATE OR REPLACE PROCEDURE public.ptest8(x text)+ - LANGUAGE sql + - BEGIN ATOMIC + - END + + pg_get_functiondef +------------------------------------------------------ + CREATE OR REPLACE PROCEDURE public.ptest8(IN x text)+ + LANGUAGE sql + + BEGIN ATOMIC + + END + (1 row) @@ -217,12 +217,105 @@ 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) + +-- check resolution of ambiguous DROP commands +CREATE PROCEDURE ptest10(IN a int, IN b int, IN c int) +LANGUAGE SQL AS $$ SELECT a + b - c $$; +\df ptest10 + List of functions + Schema | Name | Result data type | Argument data types | Type +--------+---------+------------------+-------------------------------------------+------ + public | ptest10 | | IN a integer, IN b integer, IN c integer | proc + public | ptest10 | | OUT a integer, IN b integer, IN c integer | proc +(2 rows) + +drop procedure ptest10; -- fail +ERROR: procedure name "ptest10" is not unique +HINT: Specify the argument list to select the procedure unambiguously. +drop procedure ptest10(int, int, int); -- fail +ERROR: procedure name "ptest10" is not unique +begin; +drop procedure ptest10(out int, int, int); +\df ptest10 + List of functions + Schema | Name | Result data type | Argument data types | Type +--------+---------+------------------+------------------------------------------+------ + public | ptest10 | | IN a integer, IN b integer, IN c integer | proc +(1 row) + +drop procedure ptest10(int, int, int); -- now this would work +rollback; +begin; +drop procedure ptest10(in int, int, int); +\df ptest10 + List of functions + Schema | Name | Result data type | Argument data types | Type +--------+---------+------------------+-------------------------------------------+------ + public | ptest10 | | OUT a integer, IN b integer, IN c integer | proc +(1 row) + +drop procedure ptest10(int, int, int); -- now this would work +rollback; -- various error cases CALL version(); -- error: not a procedure ERROR: version() is not a procedure @@ -242,6 +335,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..75cc0fcf2a 100644 --- a/src/test/regress/sql/create_procedure.sql +++ b/src/test/regress/sql/create_procedure.sql @@ -153,8 +153,47 @@ 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); + +-- check resolution of ambiguous DROP commands + +CREATE PROCEDURE ptest10(IN a int, IN b int, IN c int) +LANGUAGE SQL AS $$ SELECT a + b - c $$; + +\df ptest10 + +drop procedure ptest10; -- fail +drop procedure ptest10(int, int, int); -- fail +begin; +drop procedure ptest10(out int, int, int); +\df ptest10 +drop procedure ptest10(int, int, int); -- now this would work +rollback; +begin; +drop procedure ptest10(in int, int, int); +\df ptest10 +drop procedure ptest10(int, int, int); -- now this would work +rollback; -- various error cases @@ -163,6 +202,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
pgsql-hackers by date: