Reconsider the handling of procedure OUT parameters.

Commit 2453ea142 redefined pg_proc.proargtypes to include the types of
OUT parameters, for procedures only.  While that had some advantages
for implementing the SQL-spec behavior of DROP PROCEDURE, it was pretty
disastrous from a number of other perspectives.  Notably, since the
primary key of pg_proc is name + proargtypes, this made it possible to
have multiple procedures with identical names + input arguments and
differing output argument types.  That would make it impossible to call
any one of the procedures by writing just NULL (or "?", or any other
data-type-free notation) for the output argument(s).  The change also
seems likely to cause grave confusion for client applications that
examine pg_proc and expect the traditional definition of proargtypes.

Hence, revert the definition of proargtypes to what it was, and
undo a number of complications that had been added to support that.

To support the SQL-spec behavior of DROP PROCEDURE, when there are
no argmode markers in the command's parameter list, we perform the
lookup both ways (that is, matching against both proargtypes and
proallargtypes), succeeding if we get just one unique match.
In principle this could result in ambiguous-function failures
that would not happen when using only one of the two rules.
However, overloading of procedure names is thought to be a pretty
rare usage, so this shouldn't cause many problems in practice.
Postgres-specific code such as pg_dump can defend against any
possibility of such failures by being careful to specify argmodes
for all procedure arguments.

This also fixes a few other bugs in the area of CALL statements
with named parameters, and improves the documentation a little.

catversion bump forced because the representation of procedures
with OUT arguments changes.

Discussion: https://postgr.es/m/3742981.1621533210@sss.pgh.pa.us

10 files changed, 182 insertions, 59 deletions

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 16493209c63..f517a7d4aff 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -5905,9 +5905,8 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</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 c97344ff927..a3edde35039 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 38fd60128b7..c819c7bb4e3 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 9cbe2c7ceaf..033fda92ee5 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 abaa81c78b9..9e83a77b7c9 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> =&gt; <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 4f30bb93e2a..e07fc47fd31 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 bf2c6ce1aaa..4c86062f343 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 of 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
+   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>
+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();
+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,10 +209,11 @@ 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>
  </refsect1>
diff --git a/doc/src/sgml/ref/drop_routine.sgml b/doc/src/sgml/ref/drop_routine.sgml
index 6c50eb44a19..0a0a140ba0f 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,13 +93,14 @@ 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>
  </refsect1>
diff --git a/doc/src/sgml/ref/security_label.sgml b/doc/src/sgml/ref/security_label.sgml
index 407a09720b8..20a839ff0c3 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 41bcc5b79dd..3771401c01d 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>