From 6ee3261e9bb6f0aa30ec74896c57f69b0636bfdd Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 12 Mar 2024 18:16:10 -0400 Subject: [PATCH] Fix confusion about the return rowtype of SQL-language procedures. There is a very ancient hack in check_sql_fn_retval that allows a single SELECT targetlist entry of composite type to be taken as supplying all the output columns of a function returning composite. (This is grotty and fundamentally ambiguous, but it's really hard to do nested composite-returning functions without it.) As far as I know, that doesn't cause any problems in ordinary functions. It's disastrous for procedures however. All procedures that have any output parameters are labeled with prorettype RECORD, and the CALL code expects it will get back a record with one column per output parameter, regardless of whether any of those parameters is composite. Doing something else leads to an assertion failure or core dump. This is simple enough to fix: we just need to not apply that rule when considering procedures. However, that requires adding another argument to check_sql_fn_retval, which at least in principle might be getting called by external callers. Therefore, in the back branches convert check_sql_fn_retval into an ABI-preserving wrapper around a new function check_sql_fn_retval_ext. Per report from Yahor Yuzefovich. This has been broken since we implemented procedures, so back-patch to all supported branches. Discussion: https://postgr.es/m/CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com --- src/backend/catalog/pg_proc.c | 1 + src/backend/executor/functions.c | 11 +++++++++-- src/backend/optimizer/util/clauses.c | 2 ++ src/include/executor/functions.h | 1 + src/test/regress/expected/create_procedure.out | 14 +++++++++++++- src/test/regress/sql/create_procedure.sql | 11 ++++++++++- 6 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index ab2b6ca148..e6c6001f39 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -959,6 +959,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS) (void) check_sql_fn_retval(querytree_list, rettype, rettupdesc, + proc->prokind, false, NULL); } diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index a4b6e1effd..6e926ef4ee 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -746,6 +746,7 @@ init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK) fcache->returnsTuple = check_sql_fn_retval(queryTree_list, rettype, rettupdesc, + procedureStruct->prokind, false, &resulttlist); @@ -1606,6 +1607,7 @@ check_sql_fn_statements(List *queryTreeLists) bool check_sql_fn_retval(List *queryTreeLists, Oid rettype, TupleDesc rettupdesc, + char prokind, bool insertDroppedCols, List **resultTargetList) { @@ -1625,7 +1627,7 @@ check_sql_fn_retval(List *queryTreeLists, /* * If it's declared to return VOID, we don't care what's in the function. - * (This takes care of the procedure case, as well.) + * (This takes care of procedures with no output parameters, as well.) */ if (rettype == VOIDOID) return false; @@ -1780,8 +1782,13 @@ check_sql_fn_retval(List *queryTreeLists, * or not the record type really matches. For the moment we rely on * runtime type checking to catch any discrepancy, but it'd be nice to * do better at parse time. + * + * We must *not* do this for a procedure, however. Procedures with + * output parameter(s) have rettype RECORD, and the CALL code expects + * to get results corresponding to the list of output parameters, even + * when there's just one parameter that's composite. */ - if (tlistlen == 1) + if (tlistlen == 1 && prokind != PROKIND_PROCEDURE) { TargetEntry *tle = (TargetEntry *) linitial(tlist); diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 62de0225fe..d09dde210f 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -4707,6 +4707,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, querytree_list = list_make1(querytree); if (check_sql_fn_retval(list_make1(querytree_list), result_type, rettupdesc, + funcform->prokind, false, NULL)) goto fail; /* reject whole-tuple-result cases */ @@ -5253,6 +5254,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte) */ if (!check_sql_fn_retval(list_make1(querytree_list), fexpr->funcresulttype, rettupdesc, + funcform->prokind, true, NULL) && (functypclass == TYPEFUNC_COMPOSITE || functypclass == TYPEFUNC_COMPOSITE_DOMAIN || diff --git a/src/include/executor/functions.h b/src/include/executor/functions.h index 1590d1efa6..27f948c6f5 100644 --- a/src/include/executor/functions.h +++ b/src/include/executor/functions.h @@ -47,6 +47,7 @@ extern void check_sql_fn_statements(List *queryTreeLists); extern bool check_sql_fn_retval(List *queryTreeLists, Oid rettype, TupleDesc rettupdesc, + char prokind, bool insertDroppedCols, List **resultTargetList); diff --git a/src/test/regress/expected/create_procedure.out b/src/test/regress/expected/create_procedure.out index f2a677fa55..6ab09d7ec8 100644 --- a/src/test/regress/expected/create_procedure.out +++ b/src/test/regress/expected/create_procedure.out @@ -148,7 +148,19 @@ CALL ptest4a(a, b); -- error, not supported $$; ERROR: calling procedures with output arguments is not supported in SQL functions CONTEXT: SQL function "ptest4b" -DROP PROCEDURE ptest4a; +-- we used to get confused by a single output argument that is composite +CREATE PROCEDURE ptest4c(INOUT comp int8_tbl) +LANGUAGE SQL +AS $$ +SELECT ROW(1, 2); +$$; +CALL ptest4c(NULL); + comp +------- + (1,2) +(1 row) + +DROP PROCEDURE ptest4a, ptest4c; -- named and default parameters CREATE OR REPLACE PROCEDURE ptest5(a int, b text, c int default 100) LANGUAGE SQL diff --git a/src/test/regress/sql/create_procedure.sql b/src/test/regress/sql/create_procedure.sql index 35b872779e..012cdf3628 100644 --- a/src/test/regress/sql/create_procedure.sql +++ b/src/test/regress/sql/create_procedure.sql @@ -90,7 +90,16 @@ AS $$ CALL ptest4a(a, b); -- error, not supported $$; -DROP PROCEDURE ptest4a; +-- we used to get confused by a single output argument that is composite +CREATE PROCEDURE ptest4c(INOUT comp int8_tbl) +LANGUAGE SQL +AS $$ +SELECT ROW(1, 2); +$$; + +CALL ptest4c(NULL); + +DROP PROCEDURE ptest4a, ptest4c; -- named and default parameters