From 91e6c2d863b46e31f1f401a65504f968e53b9846 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 25 Sep 2026 13:42:58 -0400 Subject: [PATCH v1 06/11] Factor out parsing of flag-list GUCs with boolean compatibility log_connections is a comma-separated list of options whose flags are ORed together, and, for backwards compatibility with its earlier boolean form, also accepts a boolean value on its own. Move that parsing into a generic helper so that it can be used for other similarly structured GUCs' check hooks int he future. No behavior change for log_connections, except that the error detail for a boolean value given in a list no longer names the GUC ("Cannot specify option "on" in a list with other options."); the error message itself already names it. --- src/backend/tcop/backend_startup.c | 152 +++-------------------------- src/backend/utils/misc/guc.c | 147 ++++++++++++++++++++++++++++ src/include/utils/guc.h | 3 + 3 files changed, 163 insertions(+), 139 deletions(-) diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c index 912ad7dc957..dc0dc182903 100644 --- a/src/backend/tcop/backend_startup.c +++ b/src/backend/tcop/backend_startup.c @@ -40,7 +40,6 @@ #include "utils/memutils.h" #include "utils/ps_status.h" #include "utils/timeout.h" -#include "utils/varlena.h" /* GUCs */ bool Trace_connection_negotiation = false; @@ -64,7 +63,6 @@ static void ProcessCancelRequestPacket(Port *port, void *pkt, int pktlen); static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options); static void process_startup_packet_die(SIGNAL_ARGS); static void StartupPacketTimeoutHandler(void); -static bool validate_log_connections_options(List *elemlist, uint32 *flags); /* * Entry point for a new backend process. @@ -1006,150 +1004,26 @@ StartupPacketTimeoutHandler(void) _exit(1); } -/* - * Helper for the log_connections GUC check hook. - * - * `elemlist` is a listified version of the string input passed to the - * log_connections GUC check hook, check_log_connections(). - * check_log_connections() is responsible for cleaning up `elemlist`. - * - * validate_log_connections_options() returns false if an error was - * encountered and the GUC input could not be validated and true otherwise. - * - * `flags` returns the flags that should be stored in the log_connections GUC - * by its assign hook. - */ -static bool -validate_log_connections_options(List *elemlist, uint32 *flags) -{ - ListCell *l; - char *item; - - /* - * For backwards compatibility, we accept these tokens by themselves. - * - * Prior to PostgreSQL 18, log_connections was a boolean GUC that accepted - * any unambiguous substring of 'true', 'false', 'yes', 'no', 'on', and - * 'off'. Since log_connections became a list of strings in 18, we only - * accept complete option strings. - */ - static const struct config_enum_entry compat_options[] = { - {"off", 0}, - {"false", 0}, - {"no", 0}, - {"0", 0}, - {"on", LOG_CONNECTION_ON}, - {"true", LOG_CONNECTION_ON}, - {"yes", LOG_CONNECTION_ON}, - {"1", LOG_CONNECTION_ON}, - }; - - *flags = 0; - - /* If an empty string was passed, we're done */ - if (list_length(elemlist) == 0) - return true; - - /* - * Now check for the backwards compatibility options. They must always be - * specified on their own, so we error out if the first option is a - * backwards compatibility option and other options are also specified. - */ - item = linitial(elemlist); - - for (size_t i = 0; i < lengthof(compat_options); i++) - { - struct config_enum_entry option = compat_options[i]; - - if (pg_strcasecmp(item, option.name) != 0) - continue; - - if (list_length(elemlist) > 1) - { - GUC_check_errdetail("Cannot specify log_connections option \"%s\" in a list with other options.", - item); - return false; - } - - *flags = option.val; - return true; - } - - /* Now check the aspect options. The empty string was already handled */ - foreach(l, elemlist) - { - static const struct config_enum_entry options[] = { - {"receipt", LOG_CONNECTION_RECEIPT}, - {"authentication", LOG_CONNECTION_AUTHENTICATION}, - {"authorization", LOG_CONNECTION_AUTHORIZATION}, - {"setup_durations", LOG_CONNECTION_SETUP_DURATIONS}, - {"all", LOG_CONNECTION_ALL}, - }; - - item = lfirst(l); - for (size_t i = 0; i < lengthof(options); i++) - { - struct config_enum_entry option = options[i]; - - if (pg_strcasecmp(item, option.name) == 0) - { - *flags |= option.val; - goto next; - } - } - - GUC_check_errdetail("Invalid option \"%s\".", item); - return false; - -next: ; - } - - return true; -} - - /* * GUC check hook for log_connections + * + * Prior to PostgreSQL 18, log_connections was a boolean GUC; a boolean value + * meaning true selects LOG_CONNECTION_ON. */ bool check_log_connections(char **newval, void **extra, GucSource source) { - uint32 flags; - char *rawstring; - List *elemlist; - bool success; - - /* Need a modifiable copy of string */ - rawstring = pstrdup(*newval); - - if (!SplitIdentifierString(rawstring, ',', &elemlist)) - { - GUC_check_errdetail("Invalid list syntax in parameter \"%s\".", "log_connections"); - pfree(rawstring); - list_free(elemlist); - return false; - } - - /* Validation logic is all in the helper */ - success = validate_log_connections_options(elemlist, &flags); - - /* Time for cleanup */ - pfree(rawstring); - list_free(elemlist); - - if (!success) - return false; - - /* - * We succeeded, so allocate `extra` and save the flags there for use by - * assign_log_connections(). - */ - *extra = guc_malloc(LOG, sizeof(int)); - if (!*extra) - return false; - *((int *) *extra) = flags; + static const struct config_enum_entry options[] = { + {"receipt", LOG_CONNECTION_RECEIPT}, + {"authentication", LOG_CONNECTION_AUTHENTICATION}, + {"authorization", LOG_CONNECTION_AUTHORIZATION}, + {"setup_durations", LOG_CONNECTION_SETUP_DURATIONS}, + {"all", LOG_CONNECTION_ALL}, + {NULL, 0} + }; - return true; + return check_flag_list_guc(newval, extra, "log_connections", options, + true, LOG_CONNECTION_ON); } /* diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 21c9ccec1e2..c32929c93eb 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -51,6 +51,7 @@ #include "utils/guc_tables.h" #include "utils/memutils.h" #include "utils/timestamp.h" +#include "utils/varlena.h" #define CONFIG_FILENAME "postgresql.conf" @@ -2961,6 +2962,152 @@ config_enum_lookup_by_name(const struct config_enum *record, const char *value, return false; } +/* + * Helper for check_flag_list_guc(): compute the flags selected by 'elemlist', + * the listified value of the GUC. Returns false, with the GUC error detail + * set, if the list is invalid. + */ +static bool +validate_flag_list_guc_options(List *elemlist, + const struct config_enum_entry *options, + bool boolean_compat, int on_value, int *flags) +{ + ListCell *l; + char *item; + + /* + * For backwards compatibility with GUCs that used to be booleans, we + * accept these tokens by themselves. A boolean GUC accepts any + * unambiguous substring of 'true', 'false', 'yes', 'no', 'on', and 'off', + * but here we only accept complete option strings. + */ + static const struct config_enum_entry compat_options[] = { + {"off", false}, + {"false", false}, + {"no", false}, + {"0", false}, + {"on", true}, + {"true", true}, + {"yes", true}, + {"1", true}, + }; + + *flags = 0; + + /* If an empty string was passed, we're done */ + if (list_length(elemlist) == 0) + return true; + + /* + * If the GUC used to be a boolean, check for the backwards compatibility + * options. They must always be specified on their own, so we error out if + * the first option is a backwards compatibility option and other options + * are also specified. + */ + if (boolean_compat) + { + item = linitial(elemlist); + + for (size_t i = 0; i < lengthof(compat_options); i++) + { + if (pg_strcasecmp(item, compat_options[i].name) != 0) + continue; + + if (list_length(elemlist) > 1) + { + GUC_check_errdetail("Cannot specify option \"%s\" in a list with other options.", + item); + return false; + } + + *flags = compat_options[i].val ? on_value : 0; + return true; + } + } + + /* Now check the regular options. The empty string was already handled */ + foreach(l, elemlist) + { + const struct config_enum_entry *option; + + item = lfirst(l); + for (option = options; option->name; option++) + { + if (pg_strcasecmp(item, option->name) == 0) + break; + } + + if (!option->name) + { + GUC_check_errdetail("Invalid option \"%s\".", item); + return false; + } + + *flags |= option->val; + } + + return true; +} + +/* + * Check hook body for a list-valued GUC whose items are flags. + * + * *newval is a comma-separated list of option names from 'options', an array + * terminated by an entry with a NULL name. The flags of the listed options + * are ORed together and stored in *extra, as an int, for the GUC's assign + * hook. + * + * If 'boolean_compat' is true, for backwards compatibility with a GUC that + * used to be a boolean, a boolean value ('on', 'true', 'yes', '1', or their + * negations) is also accepted on its own, selecting 'on_value' or no flags + * respectively. Otherwise 'on_value' is ignored. + * + * 'name' is the GUC's name, for error messages. + */ +bool +check_flag_list_guc(char **newval, void **extra, const char *name, + const struct config_enum_entry *options, + bool boolean_compat, int on_value) +{ + int flags; + char *rawstring; + List *elemlist; + bool success; + + /* Need a modifiable copy of string */ + rawstring = pstrdup(*newval); + + if (!SplitIdentifierString(rawstring, ',', &elemlist)) + { + GUC_check_errdetail("Invalid list syntax in parameter \"%s\".", name); + pfree(rawstring); + list_free(elemlist); + return false; + } + + /* Validation logic is all in the helper */ + success = validate_flag_list_guc_options(elemlist, options, + boolean_compat, on_value, &flags); + + /* Time for cleanup */ + pfree(rawstring); + list_free(elemlist); + + if (!success) + return false; + + /* + * We succeeded, so allocate `extra` and save the flags there for use by + * the assign hook. + */ + *extra = guc_malloc(LOG, sizeof(int)); + if (!*extra) + return false; + *((int *) *extra) = flags; + + return true; +} + /* * Return a palloc'd string listing all the available options for an enum GUC diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 164efba6b51..6401dfcf030 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -444,6 +444,9 @@ extern bool parse_int(const char *value, int *result, int flags, const char **hintmsg); extern bool parse_real(const char *value, double *result, int flags, const char **hintmsg); +extern bool check_flag_list_guc(char **newval, void **extra, const char *name, + const struct config_enum_entry *options, + bool boolean_compat, int on_value); extern int set_config_option(const char *name, const char *value, GucContext context, GucSource source, GucAction action, bool changeVal, int elevel, -- 2.43.0