v3-0001-Allow-complex-data-for-GUC-extra.patch
text/plain
Filename: v3-0001-Allow-complex-data-for-GUC-extra.patch
Type: text/plain
Part: 0
From 5282c2a320bee615715a75a20a351c73bb2c50ef Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Sun, 14 Dec 2025 12:45:02 -0600
Subject: [PATCH v3] Allow complex data for GUC extra.
Add assertions in non-string check hook functions to enforce this.
Convert check_synchronous_standby_names, check_synchronized_standby_slots,
and check_temp_tablespaces to use the mechanism, replacing guc_malloc()
with palloc(). This provides test coverage for both SIGHUP and USERSET
contexts.
---
src/backend/commands/tablespace.c | 4 +-
src/backend/replication/slot.c | 6 +-
src/backend/replication/syncrep.c | 8 +--
src/backend/utils/misc/guc.c | 71 ++++++++++++++++++-----
src/backend/utils/misc/guc_parameters.dat | 6 +-
src/include/utils/guc.h | 1 +
6 files changed, 67 insertions(+), 29 deletions(-)
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index df31eace47..a251bcdd67 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -1284,10 +1284,8 @@ check_temp_tablespaces(char **newval, void **extra, GucSource source)
}
/* Now prepare an "extra" struct for assign_temp_tablespaces */
- myextra = guc_malloc(LOG, offsetof(temp_tablespaces_extra, tblSpcs) +
+ myextra = palloc(offsetof(temp_tablespaces_extra, tblSpcs) +
numSpcs * sizeof(Oid));
- if (!myextra)
- return false;
myextra->numSpcs = numSpcs;
memcpy(myextra->tblSpcs, tblSpcs, numSpcs * sizeof(Oid));
*extra = myextra;
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 682eccd116..59f11821e6 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2833,10 +2833,7 @@ check_synchronized_standby_slots(char **newval, void **extra, GucSource source)
foreach_ptr(char, slot_name, elemlist)
size += strlen(slot_name) + 1;
- /* GUC extra value must be guc_malloc'd, not palloc'd */
- config = (SyncStandbySlotsConfigData *) guc_malloc(LOG, size);
- if (!config)
- return false;
+ config = (SyncStandbySlotsConfigData *) palloc(size);
/* Transform the data into SyncStandbySlotsConfigData */
config->nslotnames = list_length(elemlist);
@@ -2852,6 +2849,7 @@ check_synchronized_standby_slots(char **newval, void **extra, GucSource source)
pfree(rawname);
list_free(elemlist);
+
return true;
}
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 298a3766d7..0a431618c5 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -1089,11 +1089,11 @@ check_synchronous_standby_names(char **newval, void **extra, GucSource source)
return false;
}
- /* GUC extra value must be guc_malloc'd, not palloc'd */
+ /*
+ * With GUC_EXTRA_IS_CONTEXT, we use palloc instead of guc_malloc.
+ */
pconf = (SyncRepConfigData *)
- guc_malloc(LOG, syncrep_parse_result->config_size);
- if (pconf == NULL)
- return false;
+ palloc(syncrep_parse_result->config_size);
memcpy(pconf, syncrep_parse_result, syncrep_parse_result->config_size);
*extra = pconf;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 935c235e1b..40ea2b89cb 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -771,7 +771,15 @@ set_extra_field(struct config_generic *gconf, void **field, void *newval)
/* Free old value if it's not NULL and isn't referenced anymore */
if (oldval && !extra_field_used(gconf, oldval))
- guc_free(oldval);
+ {
+ if (gconf->flags & GUC_EXTRA_IS_CONTEXT)
+ {
+ MemoryContext ctx = GetMemoryChunkContext(oldval);
+ MemoryContextDelete(ctx);
+ }
+ else
+ guc_free(oldval);
+ }
}
/*
@@ -6641,6 +6649,8 @@ static bool
call_bool_check_hook(const struct config_generic *conf, bool *newval, void **extra,
GucSource source, int elevel)
{
+ Assert(!(conf->flags & GUC_EXTRA_IS_CONTEXT));
+
/* Quick success if no hook */
if (!conf->_bool.check_hook)
return true;
@@ -6675,6 +6685,8 @@ static bool
call_int_check_hook(const struct config_generic *conf, int *newval, void **extra,
GucSource source, int elevel)
{
+ Assert(!(conf->flags & GUC_EXTRA_IS_CONTEXT));
+
/* Quick success if no hook */
if (!conf->_int.check_hook)
return true;
@@ -6709,6 +6721,8 @@ static bool
call_real_check_hook(const struct config_generic *conf, double *newval, void **extra,
GucSource source, int elevel)
{
+ Assert(!(conf->flags & GUC_EXTRA_IS_CONTEXT));
+
/* Quick success if no hook */
if (!conf->_real.check_hook)
return true;
@@ -6743,17 +6757,28 @@ static bool
call_string_check_hook(const struct config_generic *conf, char **newval, void **extra,
GucSource source, int elevel)
{
+ MemoryContext extra_ctx = NULL;
+ MemoryContext old_ctx = NULL;
volatile bool result = true;
/* Quick success if no hook */
if (!conf->_string.check_hook)
return true;
+ if (conf->flags & GUC_EXTRA_IS_CONTEXT)
+ {
+ extra_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "GUC check extra",
+ ALLOCSET_DEFAULT_SIZES);
+ MemoryContextSetIdentifier(extra_ctx, conf->name);
+ old_ctx = MemoryContextSwitchTo(extra_ctx);
+ }
+
/*
* If elevel is ERROR, or if the check_hook itself throws an elog
* (undesirable, but not always avoidable), make sure we don't leak the
* already-malloc'd newval string.
*/
PG_TRY();
{
/* Reset variables that might be set by hook */
@@ -6762,18 +6787,20 @@ call_string_check_hook(const struct config_generic *conf, char **newval, void **
GUC_check_errdetail_string = NULL;
GUC_check_errhint_string = NULL;
- if (!conf->_string.check_hook(newval, extra, source))
+ result = conf->_string.check_hook(newval, extra, source);
+
+ if (!result)
{
ereport(elevel,
(errcode(GUC_check_errcode_value),
GUC_check_errmsg_string ?
errmsg_internal("%s", GUC_check_errmsg_string) :
errmsg("invalid value for parameter \"%s\": \"%s\"",
conf->name, *newval ? *newval : ""),
GUC_check_errdetail_string ?
errdetail_internal("%s", GUC_check_errdetail_string) : 0,
GUC_check_errhint_string ?
errhint("%s", GUC_check_errhint_string) : 0));
/* Flush strings created in ErrorContext (ereport might not have) */
FlushErrorState();
result = false;
@@ -6781,18 +6808,32 @@ call_string_check_hook(const struct config_generic *conf, char **newval, void **
}
PG_CATCH();
{
+ if (conf->flags & GUC_EXTRA_IS_CONTEXT)
+ {
+ MemoryContextSwitchTo(old_ctx);
+ MemoryContextDelete(extra_ctx);
+ }
guc_free(*newval);
PG_RE_THROW();
}
PG_END_TRY();
-
+ if (conf->flags & GUC_EXTRA_IS_CONTEXT)
+ {
+ MemoryContextSwitchTo(old_ctx);
+ if (result && *extra != NULL)
+ MemoryContextSetParent(extra_ctx, GUCMemoryContext);
+ else
+ MemoryContextDelete(extra_ctx);
+ }
return result;
-}
+ }
static bool
call_enum_check_hook(const struct config_generic *conf, int *newval, void **extra,
GucSource source, int elevel)
{
+ Assert(!(conf->flags & GUC_EXTRA_IS_CONTEXT));
+
/* Quick success if no hook */
if (!conf->_enum.check_hook)
return true;
diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat
index 3b9d834907..94b519fd63 100644
--- a/src/backend/utils/misc/guc_parameters.dat
+++ b/src/backend/utils/misc/guc_parameters.dat
@@ -2816,7 +2816,7 @@
{ name => 'synchronized_standby_slots', type => 'string', context => 'PGC_SIGHUP', group => 'REPLICATION_PRIMARY',
short_desc => 'Lists streaming replication standby server replication slot names that logical WAL sender processes will wait for.',
long_desc => 'Logical WAL sender processes will send decoded changes to output plugins only after the specified replication slots have confirmed receiving WAL.',
- flags => 'GUC_LIST_INPUT',
+ flags => 'GUC_LIST_INPUT | GUC_EXTRA_IS_CONTEXT',
variable => 'synchronized_standby_slots',
boot_val => '""',
check_hook => 'check_synchronized_standby_slots',
@@ -2833,7 +2833,7 @@
{ name => 'synchronous_standby_names', type => 'string', context => 'PGC_SIGHUP', group => 'REPLICATION_PRIMARY',
short_desc => 'Number of synchronous standbys and list of names of potential synchronous ones.',
- flags => 'GUC_LIST_INPUT',
+ flags => 'GUC_LIST_INPUT | GUC_EXTRA_IS_CONTEXT',
variable => 'SyncRepStandbyNames',
boot_val => '""',
check_hook => 'check_synchronous_standby_names',
@@ -2937,7 +2937,7 @@
{ name => 'temp_tablespaces', type => 'string', context => 'PGC_USERSET', group => 'CLIENT_CONN_STATEMENT',
short_desc => 'Sets the tablespace(s) to use for temporary tables and sort files.',
long_desc => 'An empty string means use the database\'s default tablespace.',
- flags => 'GUC_LIST_INPUT | GUC_LIST_QUOTE',
+ flags => 'GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_EXTRA_IS_CONTEXT',
variable => 'temp_tablespaces',
boot_val => '""',
check_hook => 'check_temp_tablespaces',
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index f21ec37da8..c123a7af55 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -228,6 +228,7 @@ typedef enum
0x002000 /* can't set in PG_AUTOCONF_FILENAME */
#define GUC_RUNTIME_COMPUTED 0x004000 /* delay processing in 'postgres -C' */
#define GUC_ALLOW_IN_PARALLEL 0x008000 /* allow setting in parallel mode */
+#define GUC_EXTRA_IS_CONTEXT 0x010000 /* extra field is context */
#define GUC_UNIT_KB 0x01000000 /* value is in kilobytes */
#define GUC_UNIT_BLOCKS 0x02000000 /* value is in blocks */
--
2.52.0.windows.1