From e85f641b2b527255f1a66bbcce46071da9434c42 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 14 May 2024 20:19:20 -0400 Subject: [PATCH] Fix handling of polymorphic output arguments for procedures. Most of the infrastructure for procedure arguments was already okay with polymorphic output arguments, but it turns out that CallStmtResultDesc() was a few bricks shy of a load here. It thought all it needed to do was call build_function_result_tupdesc_t, but that function specifically disclaims responsibility for resolving polymorphic arguments. Failing to handle that doesn't seem to be a problem for CALL in plpgsql, but CALL from plain SQL would get errors like "cannot display a value of type anyelement", or even crash outright. In v14 and later we can simply examine the exposed types of the CallStmt.outargs nodes to get the right type OIDs. But it's a lot more complicated to fix in v12/v13, because those versions don't have CallStmt.outargs, nor do they do expand_function_arguments until ExecuteCallStmt runs. We have to duplicatively run expand_function_arguments, and then re-determine which elements of the args list are output arguments. Per bug #18463 from Drew Kimball. Back-patch to all supported versions, since it's busted in all of them. Discussion: https://postgr.es/m/18463-f8cd77e12564d8a2@postgresql.org --- src/backend/commands/functioncmds.c | 73 +++++++++++++++++++ src/pl/plpgsql/src/expected/plpgsql_call.out | 34 +++++++++ src/pl/plpgsql/src/sql/plpgsql_call.sql | 30 ++++++++ .../regress/expected/create_procedure.out | 34 +++++++++ src/test/regress/sql/create_procedure.sql | 18 +++++ 5 files changed, 189 insertions(+) diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index c24a85dc8c8..6fa414c5fa6 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -55,6 +55,7 @@ #include "executor/executor.h" #include "funcapi.h" #include "miscadmin.h" +#include "nodes/nodeFuncs.h" #include "optimizer/optimizer.h" #include "parser/parse_coerce.h" #include "parser/parse_collate.h" @@ -2327,6 +2328,78 @@ CallStmtResultDesc(CallStmt *stmt) tupdesc = build_function_result_tupdesc_t(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 + * the procedure's resolved argument expressions. 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) + { + Datum proargmodes; + bool isnull; + ArrayType *arr; + char *argmodes; + int nargs, + noutargs; + ListCell *lc; + + /* + * 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, + tuple); + + /* + * If we're here, build_function_result_tupdesc_t already validated + * that the procedure has non-null proargmodes that is the right kind + * of array, so it seems unnecessary to check again. + */ + proargmodes = SysCacheGetAttr(PROCOID, tuple, + Anum_pg_proc_proargmodes, + &isnull); + Assert(!isnull); + arr = DatumGetArrayTypeP(proargmodes); /* ensure not toasted */ + argmodes = (char *) ARR_DATA_PTR(arr); + + nargs = noutargs = 0; + foreach(lc, fexpr->args) + { + Node *arg = (Node *) lfirst(lc); + Form_pg_attribute att = TupleDescAttr(tupdesc, noutargs); + char argmode = argmodes[nargs++]; + + /* ignore non-out arguments */ + if (argmode == PROARGMODE_IN || + argmode == PROARGMODE_VARIADIC) + continue; + + TupleDescInitEntry(tupdesc, + ++noutargs, + NameStr(att->attname), + exprType(arg), + -1, + 0); + } + Assert(tupdesc->natts == noutargs); + } + ReleaseSysCache(tuple); return tupdesc; diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out index f92e108366a..83d59731359 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_call.out +++ b/src/pl/plpgsql/src/expected/plpgsql_call.out @@ -302,6 +302,40 @@ END $$; ERROR: procedure parameter "c" is an output parameter but corresponding argument is not writable CONTEXT: PL/pgSQL function inline_code_block line 5 at CALL +-- polymorphic OUT arguments +CREATE PROCEDURE test_proc12(a anyelement, INOUT b anyelement, INOUT 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.sql index d3194e44c0b..6825f59dbca 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_call.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql @@ -278,6 +278,36 @@ BEGIN END $$; +-- polymorphic OUT arguments + +CREATE PROCEDURE test_proc12(a anyelement, INOUT b anyelement, INOUT 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.out index 215abcc8c79..8bf5f9a362a 100644 --- a/src/test/regress/expected/create_procedure.out +++ b/src/test/regress/expected/create_procedure.out @@ -151,6 +151,40 @@ AS $$ SELECT NULL::int; $$; CALL ptest6(1, 2); +CREATE PROCEDURE ptest6a(inout a anyelement, inout 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, inout b anyelement, inout 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 127120278c0..ec32656c153 100644 --- a/src/test/regress/sql/create_procedure.sql +++ b/src/test/regress/sql/create_procedure.sql @@ -109,6 +109,24 @@ $$; CALL ptest6(1, 2); +CREATE PROCEDURE ptest6a(inout a anyelement, inout b anyelement) +LANGUAGE SQL +AS $$ +SELECT $1, $1; +$$; + +CALL ptest6a(1, null); +CALL ptest6a(1.1, null); + +CREATE PROCEDURE ptest6b(a anyelement, inout b anyelement, inout c anyarray) +LANGUAGE SQL +AS $$ +SELECT $1, array[$1]; +$$; + +CALL ptest6b(1, null, null); +CALL ptest6b(1.1, null, null); + -- collation assignment