From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dmitry Dolgov <9erthalion6@gmail.com>
cc: drewk@cockroachlabs.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18463: Possible bug in stored procedures with polymorphic
 OUT parameters
In-reply-to: <1124184.1715706026@sss.pgh.pa.us>
References: <18463-f8cd77e12564d8a2@postgresql.org>
 <20240514151843.yvq3wlt7n2kechv5@ddolgov.remote.csb>
 <1124184.1715706026@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 14 May 2024 13:00:26 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1341203.1715715333.0@sss.pgh.pa.us>
Date: Tue, 14 May 2024 15:35:37 -0400
Message-ID: <1341220.1715715337@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1341220.1715715337%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1341203.1715715333.1@sss.pgh.pa.us>

I wrote:
> It looks like we'd have to teach resolve_polymorphic_tupdesc how
> to get argument types out of a CallExpr, so that does not lead
> to an entirely trivial fix, but it's surely possible.

> Maybe it'd be better to not try to use build_function_result_tupdesc_t
> here at all.  It looks to me like the output argument list in the
> CallStmt is already fully polymorphically resolved, so we could just
> build a tupdesc based on that and probably save a lot of work.

Some experimentation showed that we need to return the correct
output column names in this tupdesc, so continuing to use
build_function_result_tupdesc_t seems like the easiest path for that.
However, stmt->outargs does hold nodes of the correct resolved data
types, so overwriting the atttypid's from that produces a nicely
small patch, as attached.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-polymorphic-procedure-outputs.patch";
	charset="us-ascii"
Content-ID: <1341203.1715715333.2@sss.pgh.pa.us>
Content-Description: fix-polymorphic-procedure-outputs.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/fu=
nctioncmds.c
index 9cf3fe8275..6593fd7d81 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -52,6 +52,7 @@
 #include "executor/functions.h"
 #include "funcapi.h"
 #include "miscadmin.h"
+#include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
 #include "parser/analyze.h"
 #include "parser/parse_coerce.h"
@@ -2364,5 +2365,32 @@ CallStmtResultDesc(CallStmt *stmt)
 =

 	ReleaseSysCache(tuple);
 =

+	/*
+	 * The result of build_function_result_tupdesc_t has the right column
+	 * names, but it just has the declared output argument types, which is t=
he
+	 * wrong thing in polymorphic cases.  Get the correct types by examining
+	 * stmt->outargs.  We intentionally keep the atttypmod as -1 and the
+	 * attcollation as the type's default, since that's always the appropria=
te
+	 * thing for function outputs; there's no point in considering any
+	 * additional info available from outargs.  Note that tupdesc is null if
+	 * there are no outargs.
+	 */
+	if (tupdesc)
+	{
+		Assert(tupdesc->natts =3D=3D list_length(stmt->outargs));
+		for (int i =3D 0; i < tupdesc->natts; i++)
+		{
+			Form_pg_attribute att =3D TupleDescAttr(tupdesc, i);
+			Node	   *outarg =3D (Node *) list_nth(stmt->outargs, i);
+
+			TupleDescInitEntry(tupdesc,
+							   i + 1,
+							   NameStr(att->attname),
+							   exprType(outarg),
+							   -1,
+							   0);
+		}
+	}
+
 	return tupdesc;
 }
diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql=
/src/expected/plpgsql_call.out
index ab16416c1e..17235fca91 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_call.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_call.out
@@ -409,6 +409,40 @@ END
 $$;
 NOTICE:  a: <NULL>, b: {30,7}
 NOTICE:  _a: 37, _b: 30, _c: 7
+-- polymorphic OUT arguments
+CREATE PROCEDURE test_proc12(a anyelement, OUT b anyelement, OUT c anyarr=
ay)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  RAISE NOTICE 'a: %', a;
+  b :=3D a;
+  c :=3D array[a];
+END;
+$$;
+DO $$
+DECLARE _a int; _b int; _c int[];
+BEGIN
+  _a :=3D 10;
+  CALL test_proc12(_a, _b, _c);
+  RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
+END
+$$;
+NOTICE:  a: 10
+NOTICE:  _a: 10, _b: 10, _c: {10}
+DO $$
+DECLARE _a int; _b int; _c text[];
+BEGIN
+  _a :=3D 10;
+  CALL test_proc12(_a, _b, _c);  -- error
+  RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
+END
+$$;
+ERROR:  procedure test_proc12(integer, integer, text[]) does not exist
+LINE 1: CALL test_proc12(_a, _b, _c)
+             ^
+HINT:  No procedure matches the given name and argument types. You might =
need to add explicit type casts.
+QUERY:  CALL test_proc12(_a, _b, _c)
+CONTEXT:  PL/pgSQL function inline_code_block line 5 at CALL
 -- transition variable assignment
 TRUNCATE test1;
 CREATE FUNCTION triggerfunc1() RETURNS trigger
diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/=
sql/plpgsql_call.sql
index 8efc8e823a..869d021a07 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_call.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql
@@ -375,6 +375,36 @@ BEGIN
 END
 $$;
 =

+-- polymorphic OUT arguments
+
+CREATE PROCEDURE test_proc12(a anyelement, OUT b anyelement, OUT c anyarr=
ay)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  RAISE NOTICE 'a: %', a;
+  b :=3D a;
+  c :=3D array[a];
+END;
+$$;
+
+DO $$
+DECLARE _a int; _b int; _c int[];
+BEGIN
+  _a :=3D 10;
+  CALL test_proc12(_a, _b, _c);
+  RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
+END
+$$;
+
+DO $$
+DECLARE _a int; _b int; _c text[];
+BEGIN
+  _a :=3D 10;
+  CALL test_proc12(_a, _b, _c);  -- error
+  RAISE NOTICE '_a: %, _b: %, _c: %', _a, _b, _c;
+END
+$$;
+
 =

 -- transition variable assignment
 =

diff --git a/src/test/regress/expected/create_procedure.out b/src/test/reg=
ress/expected/create_procedure.out
index 6ab09d7ec8..8a32fd960c 100644
--- a/src/test/regress/expected/create_procedure.out
+++ b/src/test/regress/expected/create_procedure.out
@@ -193,6 +193,40 @@ AS $$
 SELECT NULL::int;
 $$;
 CALL ptest6(1, 2);
+CREATE PROCEDURE ptest6a(inout a anyelement, out b anyelement)
+LANGUAGE SQL
+AS $$
+SELECT $1, $1;
+$$;
+CALL ptest6a(1, null);
+ a | b =

+---+---
+ 1 | 1
+(1 row)
+
+CALL ptest6a(1.1, null);
+  a  |  b  =

+-----+-----
+ 1.1 | 1.1
+(1 row)
+
+CREATE PROCEDURE ptest6b(a anyelement, out b anyelement, out c anyarray)
+LANGUAGE SQL
+AS $$
+SELECT $1, array[$1];
+$$;
+CALL ptest6b(1, null, null);
+ b |  c  =

+---+-----
+ 1 | {1}
+(1 row)
+
+CALL ptest6b(1.1, null, null);
+  b  |   c   =

+-----+-------
+ 1.1 | {1.1}
+(1 row)
+
 -- collation assignment
 CREATE PROCEDURE ptest7(a text, b text)
 LANGUAGE SQL
diff --git a/src/test/regress/sql/create_procedure.sql b/src/test/regress/=
sql/create_procedure.sql
index 012cdf3628..b10cf71ca4 100644
--- a/src/test/regress/sql/create_procedure.sql
+++ b/src/test/regress/sql/create_procedure.sql
@@ -131,6 +131,24 @@ $$;
 =

 CALL ptest6(1, 2);
 =

+CREATE PROCEDURE ptest6a(inout a anyelement, out b anyelement)
+LANGUAGE SQL
+AS $$
+SELECT $1, $1;
+$$;
+
+CALL ptest6a(1, null);
+CALL ptest6a(1.1, null);
+
+CREATE PROCEDURE ptest6b(a anyelement, out b anyelement, out c anyarray)
+LANGUAGE SQL
+AS $$
+SELECT $1, array[$1];
+$$;
+
+CALL ptest6b(1, null, null);
+CALL ptest6b(1.1, null, null);
+
 =

 -- collation assignment
 =


------- =_aaaaaaaaaa0--