From e77216fcb021bb19d83b348db084adfe8d918118 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 24 Feb 2022 16:54:59 +0900 Subject: [PATCH] Simplify more checks related to set-returning functions This makes more consistent the SRF-related checks in the area of PL/pgSQL, PL/Perl, PL/Tcl, pageinspect and some of the JSON worker functions, making it easier to grep for the same error patterns through the code, reducing a bit the translation work. It is worth noting that each_worker_jsonb()/each_worker() in jsonfuncs.c and pageinspect's brin_page_items() were doing a check on expectedDesc that is not required as they fetch their tuple descriptor directly from get_call_result_type(). This looks like a set of copy-paste errors that have spread over the years. This commit is a continuation of the changes begun in 07daca5, for any remaining code paths on sight. Like fcc2817, this makes the code more consistent, easing the integration of a larger patch that will refactor the way tuplestores are created and checked in a good portion of the set-returning functions present in core. I have worked my way through the changes of this patch by myself, and Ranier has proposed the same changes in a different thread in parallel, though there were some inconsistencies related in expectedDesc in what was proposed by him. Author: Michael Paquier, Ranier Vilela Discussion: https://postgr.es/m/CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com Discussion: https://postgr.es/m/CAEudQApm=AFuJjEHLBjBcJbxcw4pBMwg2sHwXyCXYcbBOj3hpg@mail.gmail.com --- contrib/pageinspect/brinfuncs.c | 3 +- src/backend/utils/adt/jsonfuncs.c | 61 ++++++++++++++++++------------- src/pl/plperl/plperl.c | 11 ++++-- src/pl/plpgsql/src/pl_exec.c | 19 +++++++--- src/pl/tcl/pltcl.c | 8 +++- 5 files changed, 63 insertions(+), 39 deletions(-) diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c index 50892b5cc20..683749a1505 100644 --- a/contrib/pageinspect/brinfuncs.c +++ b/contrib/pageinspect/brinfuncs.c @@ -148,8 +148,7 @@ brin_page_items(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("set-valued function called in context that cannot accept a set"))); - if (!(rsinfo->allowedModes & SFRM_Materialize) || - rsinfo->expectedDesc == NULL) + if (!(rsinfo->allowedModes & SFRM_Materialize)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("materialize mode required, but it is not allowed in this context"))); diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 0273f883d4f..2457061f97e 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -1927,21 +1927,19 @@ each_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname, bool as_text) rsi = (ReturnSetInfo *) fcinfo->resultinfo; - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0 || - rsi->expectedDesc == NULL) + if (!rsi || !IsA(rsi, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + if (!(rsi->allowedModes & SFRM_Materialize)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that " - "cannot accept a set"))); + errmsg("materialize mode required, but it is not allowed in this context"))); rsi->returnMode = SFRM_Materialize; if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("function returning record called in context " - "that cannot accept type record"))); + elog(ERROR, "return type must be a row type"); old_cxt = MemoryContextSwitchTo(rsi->econtext->ecxt_per_query_memory); @@ -2039,13 +2037,15 @@ each_worker(FunctionCallInfo fcinfo, bool as_text) rsi = (ReturnSetInfo *) fcinfo->resultinfo; - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0 || - rsi->expectedDesc == NULL) + if (!rsi || !IsA(rsi, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + + if (!(rsi->allowedModes & SFRM_Materialize)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that " - "cannot accept a set"))); + errmsg("materialize mode required, but it is not allowed in this context"))); rsi->returnMode = SFRM_Materialize; @@ -2227,13 +2227,16 @@ elements_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname, rsi = (ReturnSetInfo *) fcinfo->resultinfo; - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0 || + if (!rsi || !IsA(rsi, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + + if (!(rsi->allowedModes & SFRM_Materialize) || rsi->expectedDesc == NULL) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that " - "cannot accept a set"))); + errmsg("materialize mode required, but it is not allowed in this context"))); rsi->returnMode = SFRM_Materialize; @@ -2336,13 +2339,16 @@ elements_worker(FunctionCallInfo fcinfo, const char *funcname, bool as_text) rsi = (ReturnSetInfo *) fcinfo->resultinfo; - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0 || + if (!rsi || !IsA(rsi, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + + if (!(rsi->allowedModes & SFRM_Materialize) || rsi->expectedDesc == NULL) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that " - "cannot accept a set"))); + errmsg("materialize mode required, but it is not allowed in this context"))); rsi->returnMode = SFRM_Materialize; @@ -3798,12 +3804,15 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname, rsi = (ReturnSetInfo *) fcinfo->resultinfo; - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0) + if (!rsi || !IsA(rsi, ReturnSetInfo)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("set-valued function called in context that cannot accept a set"))); + + if (!(rsi->allowedModes & SFRM_Materialize)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that " - "cannot accept a set"))); + errmsg("materialize mode required, but it is not allowed in this context"))); rsi->returnMode = SFRM_Materialize; diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index b5879c29471..81d9c46e00c 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -2414,12 +2414,15 @@ plperl_func_handler(PG_FUNCTION_ARGS) if (prodesc->fn_retisset) { /* Check context before allowing the call to go through */ - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0) + if (!rsi || !IsA(rsi, ReturnSetInfo)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that " - "cannot accept a set"))); + errmsg("set-valued function called in context that cannot accept a set"))); + + if (!(rsi->allowedModes & SFRM_Materialize)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialize mode required, but it is not allowed in this context"))); } activate_interpreter(prodesc->interp); diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 70c4a752955..9674c292505 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -629,11 +629,16 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo, ReturnSetInfo *rsi = estate.rsi; /* Check caller can handle a set result */ - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0) + if (!rsi || !IsA(rsi, ReturnSetInfo)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("set-valued function called in context that cannot accept a set"))); + + if (!(rsi->allowedModes & SFRM_Materialize)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialize mode required, but it is not allowed in this context"))); + rsi->returnMode = SFRM_Materialize; /* If we produced any tuples, send back the result */ @@ -3645,13 +3650,17 @@ exec_init_tuple_store(PLpgSQL_execstate *estate) /* * Check caller can handle a set result in the way we want */ - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0 || - rsi->expectedDesc == NULL) + if (!rsi || !IsA(rsi, ReturnSetInfo)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("set-valued function called in context that cannot accept a set"))); + if (!(rsi->allowedModes & SFRM_Materialize) || + rsi->expectedDesc == NULL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialize mode required, but it is not allowed in this context"))); + /* * Switch to the right memory context and resource owner for storing the * tuplestore for return set. If we're within a subtransaction opened for diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c index ab759833db1..c5fad05e122 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -829,12 +829,16 @@ pltcl_func_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state, { ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo->resultinfo; - if (!rsi || !IsA(rsi, ReturnSetInfo) || - (rsi->allowedModes & SFRM_Materialize) == 0) + if (!rsi || !IsA(rsi, ReturnSetInfo)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("set-valued function called in context that cannot accept a set"))); + if (!(rsi->allowedModes & SFRM_Materialize)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialize mode required, but it is not allowed in this context"))); + call_state->rsi = rsi; call_state->tuple_store_cxt = rsi->econtext->ecxt_per_query_memory; call_state->tuple_store_owner = CurrentResourceOwner; -- 2.30.2