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

9 files changed, 163 insertions, 30 deletions

diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out
index c804a3ffbfe..7b156f34893 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_call.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_call.out
@@ -100,9 +100,12 @@ DECLARE
 BEGIN
     CALL test_proc6(2, x, y);
     RAISE INFO 'x = %, y = %', x, y;
+    CALL test_proc6(2, c => y, b => x);
+    RAISE INFO 'x = %, y = %', x, y;
 END;
 $$;
 INFO:  x = 6, y = 8
+INFO:  x = 12, y = 16
 DO
 LANGUAGE plpgsql
 $$
@@ -304,6 +307,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 ce8d97447d1..a68a2077c36 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 207c4424fac..0ce382e1232 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 c61a75be9bd..8108d050609 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_call.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql
@@ -93,6 +93,8 @@ DECLARE
 BEGIN
     CALL test_proc6(2, x, y);
     RAISE INFO 'x = %, y = %', x, y;
+    CALL test_proc6(2, c => y, b => x);
+    RAISE INFO 'x = %, y = %', x, y;
 END;
 $$;
 
@@ -281,6 +283,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 c3f3c8e95e5..55e1027246a 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 b7c0b5cebed..494f109b323 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 46e89b1a9e1..b0b3705ae3c 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 f0eb356cf23..e4498375ec1 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 963277e1fb8..37efbdefc23 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}]]
 $$;