From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: drewk@cockroachlabs.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18463: Possible bug in stored procedures with polymorphic OUT parameters
Date: Tue, 14 May 2024 15:35:37 -0400
Message-ID: <1341220.1715715337@sss.pgh.pa.us> (raw)
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>
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
Attachments:fix-polymorphic-procedure-outputs.patch (text/x-diff, 5.1K) downloaddiff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.cindex 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 the+ * 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 appropriate+ * 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 == list_length(stmt->outargs));+ for (int i = 0; i < tupdesc->natts; i++)+ {+ Form_pg_attribute att = TupleDescAttr(tupdesc, i);+ Node *outarg = (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.outindex 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 anyarray)+LANGUAGE plpgsql+AS $$+BEGIN+ RAISE NOTICE 'a: %', a;+ b := a;+ c := array[a];+END;+$$;+DO $$+DECLARE _a int; _b int; _c int[];+BEGIN+ _a := 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 := 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.sqlindex 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 anyarray)+LANGUAGE plpgsql+AS $$+BEGIN+ RAISE NOTICE 'a: %', a;+ b := a;+ c := array[a];+END;+$$;++DO $$+DECLARE _a int; _b int; _c int[];+BEGIN+ _a := 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 := 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/regress/expected/create_procedure.outindex 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.sqlindex 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
=
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, 9erthalion6@gmail.com, drewk@cockroachlabs.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18463: Possible bug in stored procedures with polymorphic OUT parameters
In-Reply-To: <1341220.1715715337@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox