New "single-call SRF" APIs are very confusingly named
Hi,
I unfortunately just noticed this now, just after we released...
In
commit 9e98583898c347e007958c8a09911be2ea4acfb9
Author: Michael Paquier <michael@paquier.xyz>
Date: 2022-03-07 10:26:29 +0900
Create routine able to set single-call SRFs for Materialize mode
a new helper was added:
#define SRF_SINGLE_USE_EXPECTED 0x01 /* use expectedDesc as tupdesc */
#define SRF_SINGLE_BLESS 0x02 /* validate tuple for SRF */
extern void SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags);
I think the naming here is very poor. For one, "Single" here conflicts with
ExprSingleResult which indicates "expression does not return a set",
i.e. precisely the opposite what SetSingleFuncCall() is used for. For another
the "Set" in SetSingleFuncCall makes it sound like it's function setting a
property.
Even leaving the confusion with ExprSingleResult aside, calling it "Single"
still seems very non-descriptive. I assume it's named to contrast with
init_MultiFuncCall() etc. While those are also not named greatly, they're not
typically used directly but wraped in SRF_FIRSTCALL_INIT etc.
I also quite intensely dislike SRF_SINGLE_USE_EXPECTED. It sounds like it's
saying that a single use of the SRF is expected, but that's not at all what it
means: "use expectedDesc as tupdesc".
I'm also confused by SRF_SINGLE_BLESS - the comment says "validate tuple for
SRF". BlessTupleDesc can't really be described as validation, or am I missing
something?
This IMO needs to be cleaned up.
Maybe something like InitMaterializedSRF() w/
MAT_SRF_(USE_EXPECTED_DESC|BLESS)
Greetings,
Andres Freund
On Thu, Oct 13, 2022 at 12:48:20PM -0700, Andres Freund wrote:
I unfortunately just noticed this now, just after we released...
Thanks for the feedback.
Even leaving the confusion with ExprSingleResult aside, calling it "Single"
still seems very non-descriptive. I assume it's named to contrast with
init_MultiFuncCall() etc. While those are also not named greatly, they're not
typically used directly but wraped in SRF_FIRSTCALL_INIT etc.
This is mentioned on the original thread here, as of the point that we
go through the function one single time:
/messages/by-id/Yh8SBTuzKhq7Jwda@paquier.xyz
I also quite intensely dislike SRF_SINGLE_USE_EXPECTED. It sounds like it's
saying that a single use of the SRF is expected, but that's not at all what it
means: "use expectedDesc as tupdesc".
Okay. Something like the USE_EXPECTED_DESC you are suggesting or
USE_EXPECTED_TUPLE_DESC would be fine by me.
I'm also confused by SRF_SINGLE_BLESS - the comment says "validate tuple for
SRF". BlessTupleDesc can't really be described as validation, or am I missing
something?
I'd rather keep the flag name to match the history behind this API.
How about updating the comment as of "complete tuple descriptor, for a
transient RECORD datatype", or something like that?
Maybe something like InitMaterializedSRF() w/
MAT_SRF_(USE_EXPECTED_DESC|BLESS)
Or just SetMaterializedFuncCall()? Do we always have to mention the
SRF part of it once we tell about the materialization part? The
latter sort implies the former once a function returns multiple
tuples.
I don't mind doing some renaming of all that even post-release, though
comes the question of keeping some compabitility macros for
compilation in case one uses these routines? Any existing extension
code works out-of-the-box without these new routines, so the chance of
somebody using the new stuff outside core sounds rather limited but it
does not seem worth taking a risk, either.. And this has been in the
tree for a bit more than half a year now.
--
Michael
Hi,
On 2022-10-14 10:28:34 +0900, Michael Paquier wrote:
On Thu, Oct 13, 2022 at 12:48:20PM -0700, Andres Freund wrote:
Maybe something like InitMaterializedSRF() w/
MAT_SRF_(USE_EXPECTED_DESC|BLESS)Or just SetMaterializedFuncCall()?
I think starting any function that's not a setter with Set* is very likely to
be misunderstood (SetReturning* is clearer, but long). This just reads like
you're setting the materialized function call on something.
Do we always have to mention the SRF part of it once we tell about the
materialization part?
Yes. The SRF is the important part.
The latter sort implies the former once a function returns multiple tuples.
There's lot of other other things that can be materialized.
I don't mind doing some renaming of all that even post-release, though
comes the question of keeping some compabitility macros for
compilation in case one uses these routines?
Agreed that we'd need compat. I think it'd need to be compatibility function,
not just renaming via macro, so we keep ABI compatibility.
Greetings,
Andres Freund
On Thu, Oct 13, 2022 at 3:48 PM Andres Freund <andres@anarazel.de> wrote:
I unfortunately just noticed this now, just after we released...
In
commit 9e98583898c347e007958c8a09911be2ea4acfb9
Author: Michael Paquier <michael@paquier.xyz>
Date: 2022-03-07 10:26:29 +0900Create routine able to set single-call SRFs for Materialize mode
a new helper was added:
#define SRF_SINGLE_USE_EXPECTED 0x01 /* use expectedDesc as tupdesc */
#define SRF_SINGLE_BLESS 0x02 /* validate tuple for SRF
*/
extern void SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags);
I think the naming here is very poor. For one, "Single" here conflicts
with
ExprSingleResult which indicates "expression does not return a set",
i.e. precisely the opposite what SetSingleFuncCall() is used for. For
another
the "Set" in SetSingleFuncCall makes it sound like it's function setting a
property.Even leaving the confusion with ExprSingleResult aside, calling it
"Single"
still seems very non-descriptive. I assume it's named to contrast with
init_MultiFuncCall() etc. While those are also not named greatly, they're
not
typically used directly but wraped in SRF_FIRSTCALL_INIT etc.
So, while I agree that the "Single" in SetSingleFuncCall() could be
confusing given the name of ExprSingleResult, I feel like actually all
of the names are somewhat wrong.
ExprSingleResult, ExprMultipleResult, and ExprEndResult are used as
values of ReturnSetInfo->isDone, used in value-per-call mode to indicate
whether or not a given value is the last or not. The comment on
ExprSingleResult says it means "expression does not return a set",
however, in Materialize mode (which is for functions returning a set),
isDone is supposed to be set to ExprSingleResult.
Take this code in ExecMakeFunctionResultSet()
else if (rsinfo.returnMode == SFRM_Materialize)
{
/* check we're on the same page as the function author */
if (rsinfo.isDone != ExprSingleResult)
So, isDone is used for a different purpose in value-per-call and
materialize modes (and with pretty contradictory definitions) which is
pretty confusing.
Besides that, it is not clear to me that ExprMultipleResult conveys that
the result is a member of or an element of a set. Perhaps it should be
ExprSetMemberResult and instead of using ExprSingleResult for
Materialize mode there should be another enum value that indicates "not
used" or "materialize mode". It could even be ExprSetResult -- since the
whole result is a set. Though that may be confusing since isDone is not
used for Materialize mode except to ensure "we're on the same page as
the function author".
Expr[Single|Multiple]Result aside, I do see how SINGLE/Single when used
for a helper function that does set up for SFRM_Materialize mode
functions is confusing.
The routines for SFRM_ValuePerCall all use multi, so I don't think it
was unreasonable to use single. However, I agree it would be better to
use something else (probably materialize).
The different dimensions requiring distinction are:
- returns a set (Y/N)
- called multiple times to produce a single result (Y/N)
- builds a tuplestore for result set (Y/N)
SFRM_Materialize comment says "result set instantiated in Tuplestore" --
So, I feel like the question is, does a function which returns its
entire result set in a single invocation have to do so using a
tuplestore and does one that returns part of its result set on each
invocation have to do so without a tuplestore (does each invocation have
to return a scalar or tuple)?
The current implementation may not support it, but it doesn't seem like
using a tuplestore and returning all elements of the result set vs some
of them in one invocation are alternatives.
It might be better if the SetFunctionReturnMode stuck to distinguishing
between functions returning their entire result in one invocation or
part of their result in one invocation.
That being said, the current SetSingleFuncCall() makes the tuplestore
and ensures the TupleDesc required by Materialize mode is set or
created. Since it seems only to apply to Materialize mode, I am in favor
of using "materialize" instead of "single".
Maybe something like InitMaterializedSRF() w/
MAT_SRF_(USE_EXPECTED_DESC|BLESS)
I also agree that starting the function name with Set isn't the best. I
like InitMaterializedSRF() and MAT_SRF_USE_EXPECTED_TUPLE_DESC. Are there
other kinds of descs?
Also, "single call" and "multi call" are confusing because they kind of
seem like they are describing a behavior of the function limiting the
number of times it can be called. Perhaps the multi* function names
could eventually be renamed something to convey how much of a function's
result can be expected to be produced on an invocation.
To summarize, I am in support of renaming SetSingleFuncCall() ->
InitMaterializedSRF() and SRF_SINGLE_USE_EXPECTED ->
MAT_SRF_USE_EXPECTED_TUPLE_DESC (or just DESC) as suggested elsewhere in
this thread. And I think we should eventually consider renaming the
multi* function names and consider if ExprSingleResult is a good name.
- Melanie
Melanie Plageman <melanieplageman@gmail.com> writes:
So, while I agree that the "Single" in SetSingleFuncCall() could be
confusing given the name of ExprSingleResult, I feel like actually all
of the names are somewhat wrong.
Maybe, but ExprSingleResult et al. have been there for decades and
are certainly embedded in a ton of third-party code. It's a bit
late to rename them, whether you think they're confusing or not.
Maybe we can get away with changing names introduced in v15, but
even that I'm afraid will get some pushback.
Having said that, I'd probably have used names based on "materialize"
not "single" for what this code is doing.
regards, tom lane
On Fri, Oct 14, 2022 at 05:09:46PM -0400, Melanie Plageman wrote:
To summarize, I am in support of renaming SetSingleFuncCall() ->
InitMaterializedSRF() and SRF_SINGLE_USE_EXPECTED ->
MAT_SRF_USE_EXPECTED_TUPLE_DESC (or just DESC) as suggested elsewhere in
this thread. And I think we should eventually consider renaming the
multi* function names and consider if ExprSingleResult is a good name.
As already mentioned, these have been around for years, so the impact
would be bigger. Attached is a patch for HEAD and REL_15_STABLE to
switch this stuff with new names, with what's needed for ABI
compatibility. My plan would be to keep the compatibility parts only
in 15, and drop them from HEAD.
--
Michael
Attachments:
0001-Rename-SetSingleFuncCall-its-flags.patchtext/x-diff; charset=us-asciiDownload
From 8e01bd6f7753fc21ca8567dff9a1c0ba33cb1a81 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Sat, 15 Oct 2022 11:39:36 +0900
Subject: [PATCH] Rename SetSingleFuncCall() & its flags
---
src/include/funcapi.h | 14 +++++++----
src/backend/access/transam/rmgr.c | 2 +-
src/backend/access/transam/xlogprefetcher.c | 2 +-
src/backend/commands/event_trigger.c | 4 ++--
src/backend/commands/extension.c | 6 ++---
src/backend/commands/prepare.c | 2 +-
src/backend/foreign/foreign.c | 2 +-
src/backend/replication/logical/launcher.c | 2 +-
.../replication/logical/logicalfuncs.c | 2 +-
src/backend/replication/logical/origin.c | 2 +-
src/backend/replication/slotfuncs.c | 2 +-
src/backend/replication/walsender.c | 2 +-
src/backend/storage/ipc/shmem.c | 2 +-
src/backend/utils/adt/datetime.c | 2 +-
src/backend/utils/adt/genfile.c | 4 ++--
src/backend/utils/adt/hbafuncs.c | 4 ++--
src/backend/utils/adt/jsonfuncs.c | 10 ++++----
src/backend/utils/adt/mcxtfuncs.c | 2 +-
src/backend/utils/adt/misc.c | 2 +-
src/backend/utils/adt/pgstatfuncs.c | 6 ++---
src/backend/utils/adt/varlena.c | 2 +-
src/backend/utils/fmgr/README | 2 +-
src/backend/utils/fmgr/funcapi.c | 23 +++++++++++++------
src/backend/utils/misc/guc_funcs.c | 2 +-
src/backend/utils/misc/pg_config.c | 2 +-
src/backend/utils/mmgr/portalmem.c | 2 +-
.../test_ddl_deparse/test_ddl_deparse.c | 2 +-
contrib/amcheck/verify_heapam.c | 2 +-
contrib/dblink/dblink.c | 2 +-
contrib/pageinspect/brinfuncs.c | 2 +-
contrib/pageinspect/gistfuncs.c | 4 ++--
.../pg_stat_statements/pg_stat_statements.c | 2 +-
contrib/pg_walinspect/pg_walinspect.c | 4 ++--
contrib/pgrowlocks/pgrowlocks.c | 2 +-
contrib/postgres_fdw/connection.c | 2 +-
contrib/xml2/xpath.c | 2 +-
36 files changed, 73 insertions(+), 58 deletions(-)
diff --git a/src/include/funcapi.h b/src/include/funcapi.h
index c9709f25b2..a2dec55338 100644
--- a/src/include/funcapi.h
+++ b/src/include/funcapi.h
@@ -282,7 +282,7 @@ HeapTupleGetDatum(const HeapTupleData *tuple)
* memory allocated in multi_call_memory_ctx, but holding file descriptors or
* other non-memory resources open across calls is a bug. SRFs that need
* such resources should not use these macros, but instead populate a
- * tuplestore during a single call, as set up by SetSingleFuncCall() (see
+ * tuplestore during a single call, as set up by InitMaterializedSRF() (see
* fmgr/README). Alternatively, set up a callback to release resources
* at query shutdown, using RegisterExprContextCallback().
*
@@ -291,9 +291,15 @@ HeapTupleGetDatum(const HeapTupleData *tuple)
/* from funcapi.c */
-/* flag bits for SetSingleFuncCall() */
-#define SRF_SINGLE_USE_EXPECTED 0x01 /* use expectedDesc as tupdesc */
-#define SRF_SINGLE_BLESS 0x02 /* validate tuple for SRF */
+/* flag bits for InitMaterializedSRF() */
+#define MAT_SRF_USE_EXPECTED_DESC 0x01 /* use expectedDesc as tupdesc */
+#define MAT_SRF_BLESS 0x02 /* complete tuple descriptor, for
+ * a transient RECORD datatype */
+extern void InitMaterializedSRF(FunctionCallInfo fcinfo, bits32 flags);
+
+/* Compatibility declarations, for v15 */
+#define SRF_SINGLE_USE_EXPECTED MAT_SRF_USE_EXPECTED_DESC
+#define SRF_SINGLE_BLESS MAT_SRF_BLESS
extern void SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags);
extern FuncCallContext *init_MultiFuncCall(PG_FUNCTION_ARGS);
diff --git a/src/backend/access/transam/rmgr.c b/src/backend/access/transam/rmgr.c
index 3b6de3aa04..6bb4de387f 100644
--- a/src/backend/access/transam/rmgr.c
+++ b/src/backend/access/transam/rmgr.c
@@ -145,7 +145,7 @@ pg_get_wal_resource_managers(PG_FUNCTION_ARGS)
Datum values[PG_GET_RESOURCE_MANAGERS_COLS];
bool nulls[PG_GET_RESOURCE_MANAGERS_COLS] = {0};
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
for (int rmid = 0; rmid <= RM_MAX_ID; rmid++)
{
diff --git a/src/backend/access/transam/xlogprefetcher.c b/src/backend/access/transam/xlogprefetcher.c
index 1cbac4b7f6..0cf03945ee 100644
--- a/src/backend/access/transam/xlogprefetcher.c
+++ b/src/backend/access/transam/xlogprefetcher.c
@@ -834,7 +834,7 @@ pg_stat_get_recovery_prefetch(PG_FUNCTION_ARGS)
Datum values[PG_STAT_GET_RECOVERY_PREFETCH_COLS];
bool nulls[PG_STAT_GET_RECOVERY_PREFETCH_COLS];
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
for (int i = 0; i < PG_STAT_GET_RECOVERY_PREFETCH_COLS; ++i)
nulls[i] = false;
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 441f29d684..8d36b66488 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1305,7 +1305,7 @@ pg_event_trigger_dropped_objects(PG_FUNCTION_ARGS)
"pg_event_trigger_dropped_objects()")));
/* Build tuplestore to hold the result rows */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
slist_foreach(iter, &(currentEventTriggerState->SQLDropList))
{
@@ -1832,7 +1832,7 @@ pg_event_trigger_ddl_commands(PG_FUNCTION_ARGS)
"pg_event_trigger_ddl_commands()")));
/* Build tuplestore to hold the result rows */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
foreach(lc, currentEventTriggerState->commandList)
{
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 6b6720c690..1a62e5dac5 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1946,7 +1946,7 @@ pg_available_extensions(PG_FUNCTION_ARGS)
struct dirent *de;
/* Build tuplestore to hold the result rows */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
location = get_extension_control_directory();
dir = AllocateDir(location);
@@ -2026,7 +2026,7 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
struct dirent *de;
/* Build tuplestore to hold the result rows */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
location = get_extension_control_directory();
dir = AllocateDir(location);
@@ -2281,7 +2281,7 @@ pg_extension_update_paths(PG_FUNCTION_ARGS)
check_valid_extension_name(NameStr(*extname));
/* Build tuplestore to hold the result rows */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* Read the extension's control file */
control = read_extension_control_file(NameStr(*extname));
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index c4b54d0547..9e29584d93 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -672,7 +672,7 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
* We put all the tuples into a tuplestore in one scan of the hashtable.
* This avoids any issue of the hashtable possibly changing between calls.
*/
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* hash table might be uninitialized */
if (prepared_queries)
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 353e20a0cf..56fcb8edf1 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -517,7 +517,7 @@ pg_options_to_table(PG_FUNCTION_ARGS)
rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
/* prepare the result set */
- SetSingleFuncCall(fcinfo, SRF_SINGLE_USE_EXPECTED);
+ InitMaterializedSRF(fcinfo, MAT_SRF_USE_EXPECTED_DESC);
foreach(cell, options)
{
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 3bbd522724..ff57421da6 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -930,7 +930,7 @@ pg_stat_get_subscription(PG_FUNCTION_ARGS)
int i;
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* Make sure we get consistent view of the workers. */
LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 7fa2b2cba7..5c23178570 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -188,7 +188,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
}
}
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
p->tupstore = rsinfo->setResult;
p->tupdesc = rsinfo->setDesc;
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index f19b72ff35..f134e44878 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -1503,7 +1503,7 @@ pg_show_replication_origin_status(PG_FUNCTION_ARGS)
/* we want to return 0 rows if slot is set to zero */
replorigin_check_prerequisites(false, true);
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* prevent slots from being concurrently dropped */
LWLockAcquire(ReplicationOriginLock, LW_SHARED);
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index ca945994ef..16a3527903 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -242,7 +242,7 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
* name, which shouldn't contain anything particularly sensitive.
*/
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
currlsn = GetXLogWriteRecPtr();
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index e9ba500a15..2193dcaec6 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3459,7 +3459,7 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
int num_standbys;
int i;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/*
* Get the currently active synchronous standbys. This could be out of
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index c1279960cd..10be765fb7 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -543,7 +543,7 @@ pg_get_shmem_allocations(PG_FUNCTION_ARGS)
Datum values[PG_GET_SHMEM_SIZES_COLS];
bool nulls[PG_GET_SHMEM_SIZES_COLS];
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
LWLockAcquire(ShmemIndexLock, LW_SHARED);
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 74b6807098..8cd10ab204 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -5058,7 +5058,7 @@ pg_timezone_names(PG_FUNCTION_ARGS)
Interval *resInterval;
struct pg_itm_in itm_in;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* initialize timezone scanning code */
tzenum = pg_tzenumerate_start();
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 2f1e907a10..ab6f67f874 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -561,7 +561,7 @@ pg_ls_dir(PG_FUNCTION_ARGS)
include_dot_dirs = PG_GETARG_BOOL(2);
}
- SetSingleFuncCall(fcinfo, SRF_SINGLE_USE_EXPECTED);
+ InitMaterializedSRF(fcinfo, MAT_SRF_USE_EXPECTED_DESC);
dirdesc = AllocateDir(location);
if (!dirdesc)
@@ -619,7 +619,7 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
DIR *dirdesc;
struct dirent *de;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/*
* Now walk the directory. Note that we must do this within a single SRF
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 9e5794071c..cbbe44ff13 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -421,7 +421,7 @@ pg_hba_file_rules(PG_FUNCTION_ARGS)
* also more efficient than having to look up our current position in the
* parsed list every time.
*/
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* Fill the tuplestore */
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
@@ -554,7 +554,7 @@ pg_ident_file_mappings(PG_FUNCTION_ARGS)
* also more efficient than having to look up our current position in the
* parsed list every time.
*/
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* Fill the tuplestore */
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index fd0d355789..9ca584513a 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -1921,7 +1921,7 @@ each_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname, bool as_text)
funcname)));
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo, SRF_SINGLE_BLESS);
+ InitMaterializedSRF(fcinfo, MAT_SRF_BLESS);
tmp_cxt = AllocSetContextCreate(CurrentMemoryContext,
"jsonb_each temporary cxt",
@@ -2001,7 +2001,7 @@ each_worker(FunctionCallInfo fcinfo, bool as_text)
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo, SRF_SINGLE_BLESS);
+ InitMaterializedSRF(fcinfo, MAT_SRF_BLESS);
state->tuple_store = rsi->setResult;
state->ret_tdesc = rsi->setDesc;
@@ -2164,8 +2164,8 @@ elements_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname,
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo,
- SRF_SINGLE_USE_EXPECTED | SRF_SINGLE_BLESS);
+ InitMaterializedSRF(fcinfo,
+ MAT_SRF_USE_EXPECTED_DESC | MAT_SRF_BLESS);
tmp_cxt = AllocSetContextCreate(CurrentMemoryContext,
"jsonb_array_elements temporary cxt",
@@ -2243,7 +2243,7 @@ elements_worker(FunctionCallInfo fcinfo, const char *funcname, bool as_text)
state = palloc0(sizeof(ElementsState));
sem = palloc0(sizeof(JsonSemAction));
- SetSingleFuncCall(fcinfo, SRF_SINGLE_USE_EXPECTED | SRF_SINGLE_BLESS);
+ InitMaterializedSRF(fcinfo, MAT_SRF_USE_EXPECTED_DESC | MAT_SRF_BLESS);
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
state->tuple_store = rsi->setResult;
state->ret_tdesc = rsi->setDesc;
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index bb7cc94024..04b7aa2a77 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -121,7 +121,7 @@ pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
TopMemoryContext, NULL, 0);
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 6c45fd2007..9c13251231 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -208,7 +208,7 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
DIR *dirdesc;
struct dirent *de;
- SetSingleFuncCall(fcinfo, SRF_SINGLE_USE_EXPECTED);
+ InitMaterializedSRF(fcinfo, MAT_SRF_USE_EXPECTED_DESC);
if (tablespaceOid == GLOBALTABLESPACE_OID)
{
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 85ac3e3f04..96bffc0f2a 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -502,7 +502,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid command name: \"%s\"", cmd)));
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* 1-based index */
for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
@@ -559,7 +559,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0);
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* 1-based index */
for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
@@ -1800,7 +1800,7 @@ pg_stat_get_slru(PG_FUNCTION_ARGS)
int i;
PgStat_SLRUStats *stats;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* request SLRU stats from the cumulative stats system */
stats = pgstat_fetch_slru();
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 1f6e090821..c5e7ee7ca2 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4810,7 +4810,7 @@ text_to_table(PG_FUNCTION_ARGS)
SplitTextOutputData tstate;
tstate.astate = NULL;
- SetSingleFuncCall(fcinfo, SRF_SINGLE_USE_EXPECTED);
+ InitMaterializedSRF(fcinfo, MAT_SRF_USE_EXPECTED_DESC);
tstate.tupstore = rsi->setResult;
tstate.tupdesc = rsi->setDesc;
diff --git a/src/backend/utils/fmgr/README b/src/backend/utils/fmgr/README
index 9d8848106d..4b2a5df285 100644
--- a/src/backend/utils/fmgr/README
+++ b/src/backend/utils/fmgr/README
@@ -305,7 +305,7 @@ If available, the expected tuple descriptor is passed in ReturnSetInfo;
in other contexts the expectedDesc field will be NULL. The function need
not pay attention to expectedDesc, but it may be useful in special cases.
-SetSingleFuncCall() is a helper function able to setup the function's
+InitMaterializedSRF() is a helper function able to setup the function's
ReturnSetInfo for a single call, filling in the Tuplestore and the
TupleDesc with the proper configuration for Materialize mode.
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index a1fe50ffca..a277dd8841 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -57,7 +57,16 @@ static TypeFuncClass get_type_func_class(Oid typid, Oid *base_typeid);
/*
- * SetSingleFuncCall
+ * Compatibility function for v15.
+ */
+void
+SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags)
+{
+ InitMaterializedSRF(fcinfo, flags);
+}
+
+/*
+ * InitMaterializedSRF
*
* Helper function to build the state of a set-returning function used
* in the context of a single call with materialize mode. This code
@@ -65,15 +74,15 @@ static TypeFuncClass get_type_func_class(Oid typid, Oid *base_typeid);
* the TupleDesc used with the function and stores them into the
* function's ReturnSetInfo.
*
- * "flags" can be set to SRF_SINGLE_USE_EXPECTED, to use the tuple
+ * "flags" can be set to MAT_SRF_USE_EXPECTED_DESC, to use the tuple
* descriptor coming from expectedDesc, which is the tuple descriptor
- * expected by the caller. SRF_SINGLE_BLESS can be set to complete the
+ * expected by the caller. MAT_SRF_BLESS can be set to complete the
* information associated to the tuple descriptor, which is necessary
* in some cases where the tuple descriptor comes from a transient
* RECORD datatype.
*/
void
-SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags)
+InitMaterializedSRF(FunctionCallInfo fcinfo, bits32 flags)
{
bool random_access;
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
@@ -88,7 +97,7 @@ SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("set-valued function called in context that cannot accept a set")));
if (!(rsinfo->allowedModes & SFRM_Materialize) ||
- ((flags & SRF_SINGLE_USE_EXPECTED) != 0 && rsinfo->expectedDesc == NULL))
+ ((flags & MAT_SRF_USE_EXPECTED_DESC) != 0 && rsinfo->expectedDesc == NULL))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("materialize mode required, but it is not allowed in this context")));
@@ -101,7 +110,7 @@ SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags)
old_context = MemoryContextSwitchTo(per_query_ctx);
/* build a tuple descriptor for our result type */
- if ((flags & SRF_SINGLE_USE_EXPECTED) != 0)
+ if ((flags & MAT_SRF_USE_EXPECTED_DESC) != 0)
stored_tupdesc = CreateTupleDescCopy(rsinfo->expectedDesc);
else
{
@@ -110,7 +119,7 @@ SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags)
}
/* If requested, bless the tuple descriptor */
- if ((flags & SRF_SINGLE_BLESS) != 0)
+ if ((flags & MAT_SRF_BLESS) != 0)
BlessTupleDesc(stored_tupdesc);
random_access = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
diff --git a/src/backend/utils/misc/guc_funcs.c b/src/backend/utils/misc/guc_funcs.c
index fb763df5fe..108b3bd129 100644
--- a/src/backend/utils/misc/guc_funcs.c
+++ b/src/backend/utils/misc/guc_funcs.c
@@ -996,7 +996,7 @@ show_all_file_settings(PG_FUNCTION_ARGS)
conf = ProcessConfigFileInternal(PGC_SIGHUP, false, DEBUG3);
/* Build a tuplestore to return our results in */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* Process the results and create a tuplestore */
for (seqno = 1; conf != NULL; conf = conf->next, seqno++)
diff --git a/src/backend/utils/misc/pg_config.c b/src/backend/utils/misc/pg_config.c
index d9e18caf44..581965395d 100644
--- a/src/backend/utils/misc/pg_config.c
+++ b/src/backend/utils/misc/pg_config.c
@@ -30,7 +30,7 @@ pg_config(PG_FUNCTION_ARGS)
int i = 0;
/* initialize our tuplestore */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
configdata = get_configdata(my_exec_path, &configdata_len);
for (i = 0; i < configdata_len; i++)
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 3a161bdb88..c3e95346b6 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -1139,7 +1139,7 @@ pg_cursor(PG_FUNCTION_ARGS)
* We put all the tuples into a tuplestore in one scan of the hashtable.
* This avoids any issue of the hashtable possibly changing between calls.
*/
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
hash_seq_init(&hash_seq, PortalHashTable);
while ((hentry = hash_seq_search(&hash_seq)) != NULL)
diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
index 133594999b..7e9e443306 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -93,7 +93,7 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS)
if (cmd->type != SCT_AlterTable)
elog(ERROR, "command is not ALTER TABLE");
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
if (cmd->d.alterTable.subcmds == NIL)
elog(ERROR, "empty alter table subcommand list");
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 83dc728011..b72a5c96d1 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -278,7 +278,7 @@ verify_heapam(PG_FUNCTION_ARGS)
ctx.attnum = -1;
/* Construct the tuplestore and tuple descriptor */
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
ctx.tupdesc = rsinfo->setDesc;
ctx.tupstore = rsinfo->setResult;
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 9eef417c47..9202c35847 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -1933,7 +1933,7 @@ dblink_get_notify(PG_FUNCTION_ARGS)
else
conn = pconn->conn;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
PQconsumeInput(conn);
while ((notify = PQnotifies(conn)) != NULL)
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index f4c959ecab..12a7217038 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -147,7 +147,7 @@ brin_page_items(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to use raw page functions")));
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
indexRel = index_open(indexRelid, AccessShareLock);
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index d0a34a3375..f15714842a 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -127,7 +127,7 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to use raw page functions")));
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
page = get_page_from_raw(raw_page);
@@ -211,7 +211,7 @@ gist_page_items(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to use raw page functions")));
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* Open the relation */
indexRel = index_open(indexRelid, AccessShareLock);
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 73439c0199..e5aa429995 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1552,7 +1552,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("pg_stat_statements must be loaded via shared_preload_libraries")));
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/*
* Check we have the expected number of output arguments. Aside from
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index 38fb4106da..beba4788c7 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -330,7 +330,7 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
Datum values[PG_GET_WAL_RECORDS_INFO_COLS] = {0};
bool nulls[PG_GET_WAL_RECORDS_INFO_COLS] = {0};
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
xlogreader = InitXLogReaderState(start_lsn);
@@ -548,7 +548,7 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
Datum values[PG_GET_WAL_STATS_COLS] = {0};
bool nulls[PG_GET_WAL_STATS_COLS] = {0};
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
xlogreader = InitXLogReaderState(start_lsn);
diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c
index 1d4d4965ac..c543277b7c 100644
--- a/contrib/pgrowlocks/pgrowlocks.c
+++ b/contrib/pgrowlocks/pgrowlocks.c
@@ -75,7 +75,7 @@ pgrowlocks(PG_FUNCTION_ARGS)
AclResult aclresult;
char **values;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* Access the table */
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 939d114f02..f0c45b00db 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -1668,7 +1668,7 @@ postgres_fdw_get_connections(PG_FUNCTION_ARGS)
HASH_SEQ_STATUS scan;
ConnCacheEntry *entry;
- SetSingleFuncCall(fcinfo, 0);
+ InitMaterializedSRF(fcinfo, 0);
/* If cache doesn't exist, we return no records */
if (!ConnectionHash)
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index b8ee757674..a692dc6be8 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -511,7 +511,7 @@ xpath_table(PG_FUNCTION_ARGS)
PgXmlErrorContext *xmlerrcxt;
volatile xmlDocPtr doctree = NULL;
- SetSingleFuncCall(fcinfo, SRF_SINGLE_USE_EXPECTED);
+ InitMaterializedSRF(fcinfo, MAT_SRF_USE_EXPECTED_DESC);
/* must have at least one output column (for the pkey) */
if (rsinfo->setDesc->natts < 1)
--
2.37.2
On Fri, Oct 14, 2022 at 7:41 PM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Oct 14, 2022 at 05:09:46PM -0400, Melanie Plageman wrote:
To summarize, I am in support of renaming SetSingleFuncCall() ->
InitMaterializedSRF() and SRF_SINGLE_USE_EXPECTED ->
MAT_SRF_USE_EXPECTED_TUPLE_DESC (or just DESC) as suggested elsewhere in
this thread. And I think we should eventually consider renaming the
multi* function names and consider if ExprSingleResult is a good name.As already mentioned, these have been around for years, so the impact
would be bigger.
That makes sense.
Attached is a patch for HEAD and REL_15_STABLE to
switch this stuff with new names, with what's needed for ABI
compatibility. My plan would be to keep the compatibility parts only
in 15, and drop them from HEAD.
- * SetSingleFuncCall
+ * Compatibility function for v15.
+ */
+void
+SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags)
+{
+ InitMaterializedSRF(fcinfo, flags);
+}
+Any reason not to use a macro?
- Melanie
Hi,
On 2022-10-16 13:22:41 -0700, Melanie Plageman wrote:
On Fri, Oct 14, 2022 at 7:41 PM Michael Paquier <michael@paquier.xyz> wrote: - * SetSingleFuncCall + * Compatibility function for v15. + */ +void +SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags) +{ + InitMaterializedSRF(fcinfo, flags); +} +Any reason not to use a macro?
Yes - it'd introduce an ABI break, i.e. an already compiled extension
referencing SetSingleFuncCall() wouldn't fail to load into an upgraded sever,
due to the reference to the SetSingleFuncCall, which wouldn't exist anymore.
Greetings,
Andres Freund
Hi,
On 2022-10-15 11:41:08 +0900, Michael Paquier wrote:
Attached is a patch for HEAD and REL_15_STABLE to switch this stuff with new
names, with what's needed for ABI compatibility. My plan would be to keep
the compatibility parts only in 15, and drop them from HEAD. -- Michael
Looks reasonable to me. Thanks for working on this.
-/* flag bits for SetSingleFuncCall() */ -#define SRF_SINGLE_USE_EXPECTED 0x01 /* use expectedDesc as tupdesc */ -#define SRF_SINGLE_BLESS 0x02 /* validate tuple for SRF */ +/* flag bits for InitMaterializedSRF() */ +#define MAT_SRF_USE_EXPECTED_DESC 0x01 /* use expectedDesc as tupdesc */ +#define MAT_SRF_BLESS 0x02 /* complete tuple descriptor, for + * a transient RECORD datatype */
I don't really know what "complete tuple descriptor" means. BlessTupleDesc()
does say "make a completed tuple descriptor useful for SRFs" - but I don't
think that means that Bless* makes them complete, but that they *have* to be
complete to be blessed.
@@ -2164,8 +2164,8 @@ elements_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname,
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo, - SRF_SINGLE_USE_EXPECTED | SRF_SINGLE_BLESS); + InitMaterializedSRF(fcinfo, + MAT_SRF_USE_EXPECTED_DESC | MAT_SRF_BLESS);
Already was the case, so maybe not worth mucking with: Why the newline here,
but not in other cases?
Greetings,
Andres Freund
On Sun, Oct 16, 2022 at 03:04:43PM -0700, Andres Freund wrote:
Yes - it'd introduce an ABI break, i.e. an already compiled extension
referencing SetSingleFuncCall() wouldn't fail to load into an upgraded sever,
due to the reference to the SetSingleFuncCall, which wouldn't exist anymore.
Note that this layer should just be removed on HEAD. Once an
extension catches up with the new name, they would not even need to
play with PG_VERSION_NUM even for a new version compiled with
REL_15_STABLE.
--
Michael
On Sun, Oct 16, 2022 at 03:09:14PM -0700, Andres Freund wrote:
On 2022-10-15 11:41:08 +0900, Michael Paquier wrote:
-/* flag bits for SetSingleFuncCall() */ -#define SRF_SINGLE_USE_EXPECTED 0x01 /* use expectedDesc as tupdesc */ -#define SRF_SINGLE_BLESS 0x02 /* validate tuple for SRF */ +/* flag bits for InitMaterializedSRF() */ +#define MAT_SRF_USE_EXPECTED_DESC 0x01 /* use expectedDesc as tupdesc */ +#define MAT_SRF_BLESS 0x02 /* complete tuple descriptor, for + * a transient RECORD datatype */I don't really know what "complete tuple descriptor" means. BlessTupleDesc()
does say "make a completed tuple descriptor useful for SRFs" - but I don't
think that means that Bless* makes them complete, but that they *have* to be
complete to be blessed.
That's just assign_record_type_typmod(), which would make sure to fill
the cache for a RECORD tupdesc. How about "fill the cache with the
information of the tuple descriptor type, for a transient RECORD
datatype"? If you have a better, somewhat less confusing, idea, I am
open to suggestions.
@@ -2164,8 +2164,8 @@ elements_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname,
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
- SetSingleFuncCall(fcinfo, - SRF_SINGLE_USE_EXPECTED | SRF_SINGLE_BLESS); + InitMaterializedSRF(fcinfo, + MAT_SRF_USE_EXPECTED_DESC | MAT_SRF_BLESS);Already was the case, so maybe not worth mucking with: Why the newline here,
but not in other cases?
Yeah, that's fine as well.
--
Michael
On Mon, Oct 17, 2022 at 10:13:33AM +0900, Michael Paquier wrote:
That's just assign_record_type_typmod(), which would make sure to fill
the cache for a RECORD tupdesc. How about "fill the cache with the
information of the tuple descriptor type, for a transient RECORD
datatype"? If you have a better, somewhat less confusing, idea, I am
open to suggestions.
At the end, I was unhappy with this formulation, so I have just
mentioned what the top of BlessTupleDesc() tells, with the name of
the function used on the tuple descriptor when the flag is activated
by the init call.
The compatibility functions have been removed from HEAD, by the way.
--
Michael