From 668c6e24e58f1fd9a75bb6040990d26f89a52513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Fri, 6 Feb 2026 10:24:29 +0100 Subject: [PATCH v10 2/2] review --- doc/src/sgml/config.sgml | 18 ++-- src/backend/commands/variable.c | 127 +++++++++++------------- src/backend/postmaster/launch_backend.c | 2 +- src/backend/utils/init/miscinit.c | 2 +- src/backend/utils/misc/guc_tables.c | 14 +-- src/include/postmaster/proctypelist.h | 38 +++---- src/test/regress/expected/guc.out | 8 +- 7 files changed, 93 insertions(+), 116 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index cbc748a3636..984194ca758 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7090,15 +7090,15 @@ local0.* /var/log/postgresql - Controls which message - levels are written to the server log. - Valid values are a comma-separated list of type:level - and a single level. The list allows it to use - different levels per process type. Only the single level - is mandatory (order does not matter) and it is assigned to the process - types that are not specified in the list. - Valid process types are listed in the table below, each corresponding to - either postmaster, an auxiliary process type or a backend. + Controls which + message levels + are written to the server log. Valid values are a comma-separated + list of zero or more + process type:level + entries and exactly one mandatory + level entry, + which is the default for process types not listed. + Valid process types are listed in the table below. archiver autovacuum diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index a450254d1fa..97b9a6b5964 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -1279,12 +1279,12 @@ check_standard_conforming_strings(bool *newval, void **extra, GucSource source) /* * GUC check_hook for log_min_messages * - * The parsing consists of a comma-separated list of TYPE:LEVEL elements. TYPE - * is log_min_messages_process_types. LEVEL is server_message_level_options. A - * single LEVEL element should be part of this list and it is applied as a - * final step to the process types that are not specified. For backward - * compatibility, the old syntax is still accepted and it means to apply the - * LEVEL for all process types. + * This value is parsed as a comma-separated list of zero or more TYPE:LEVEL + * elements. For each element, TYPE corresponds to a bk_category value (see + * postmaster/proctypelist.h), and LEVEL is one of log_min_message_lvls. + * + * In addition, there must be a single LEVEL element (with no TYPE part) + * which sets the default level for process types that aren't specified. */ bool check_log_min_messages(char **newval, void **extra, GucSource source) @@ -1293,22 +1293,21 @@ check_log_min_messages(char **newval, void **extra, GucSource source) List *elemlist; StringInfoData buf; char *result; - bool first = true; int newlevel[BACKEND_NUM_TYPES]; - bool assigned[BACKEND_NUM_TYPES]; + bool assigned[BACKEND_NUM_TYPES] = {0}; int genericlevel = -1; /* -1 means not assigned */ - /* Initialize the array. */ - for (int i = 0; i < BACKEND_NUM_TYPES; i++) - { - newlevel[i] = WARNING; - assigned[i] = false; - } + const char *const process_types[] = { +#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach) \ + [bktype] = bkcategory, +#include "postmaster/proctypelist.h" +#undef PG_PROCTYPE + }; /* Need a modifiable copy of string. */ rawstring = guc_strdup(LOG, *newval); - /* Parse string into list of identifiers. */ + /* Parse the string into a list. */ if (!SplitGUCList(rawstring, ',', &elemlist)) { /* syntax error in list */ @@ -1321,27 +1320,22 @@ check_log_min_messages(char **newval, void **extra, GucSource source) /* Validate and assign log level and process type. */ foreach_ptr(char, tok, elemlist) { - char *sep; - const struct config_enum_entry *entry; + char *sep = strchr(tok, ':'); /* - * Check whether there is a process type following the log level. If - * there is no separator, it means this is the generic log level. The - * generic log level will be assigned to the process types that were - * not informed. + * If there's no ':' separator in the entry, this is the default + * value. Otherwise it's a process type-specific entry. */ - sep = strchr(tok, ':'); if (sep == NULL) { + const struct config_enum_entry *entry; bool found = false; /* Reject duplicates for generic log level. */ if (genericlevel != -1) { - GUC_check_errdetail("Generic log level was already assigned."); - guc_free(rawstring); - list_free(elemlist); - return false; + GUC_check_errdetail("Redundant specification of default log level."); + goto lmm_fail; } /* Is the log level valid? */ @@ -1358,9 +1352,7 @@ check_log_min_messages(char **newval, void **extra, GucSource source) if (!found) { GUC_check_errdetail("Unrecognized log level: \"%s\".", tok); - guc_free(rawstring); - list_free(elemlist); - return false; + goto lmm_fail; } } else @@ -1368,23 +1360,19 @@ check_log_min_messages(char **newval, void **extra, GucSource source) char *loglevel; char *ptype; bool found = false; + int level; + const struct config_enum_entry *entry; - ptype = guc_malloc(LOG, (sep - tok) + 1); - if (!ptype) - { - guc_free(rawstring); - list_free(elemlist); - return false; - } - memcpy(ptype, tok, sep - tok); - ptype[sep - tok] = '\0'; + ptype = tok; loglevel = sep + 1; + *sep = '\0'; /* Is the log level valid? */ for (entry = server_message_level_options; entry && entry->name; entry++) { if (pg_strcasecmp(entry->name, loglevel) == 0) { + level = entry->val; found = true; break; } @@ -1392,50 +1380,49 @@ check_log_min_messages(char **newval, void **extra, GucSource source) if (!found) { - GUC_check_errdetail("Unrecognized log level: \"%s\".", loglevel); - guc_free(ptype); - guc_free(rawstring); - list_free(elemlist); - return false; + GUC_check_errdetail("Unrecognized log level for process type \"%s\": \"%s\".", + ptype, loglevel); + goto lmm_fail; } - /* - * Is the process type name valid? There might be multiple entries - * per process type, don't bail out because it can assign the - * value for multiple entries. - */ + /* Is the process type name valid and unique? */ found = false; for (int i = 0; i < BACKEND_NUM_TYPES; i++) { - if (pg_strcasecmp(log_min_messages_process_types[i], ptype) == 0) + if (pg_strcasecmp(process_types[i], ptype) == 0) { /* Reject duplicates for a process type. */ if (assigned[i]) { - GUC_check_errdetail("Process type \"%s\" was already assigned.", ptype); - guc_free(ptype); - guc_free(rawstring); - list_free(elemlist); - return false; + GUC_check_errdetail("Redundant log level specification for process type \"%s\".", + ptype); + goto lmm_fail; } - newlevel[i] = entry->val; + newlevel[i] = level; assigned[i] = true; found = true; + break; } } if (!found) { GUC_check_errdetail("Unrecognized process type: \"%s\".", ptype); - guc_free(ptype); - guc_free(rawstring); - list_free(elemlist); - return false; + goto lmm_fail; } - guc_free(ptype); + /* Put the separator back in place */ + *sep = ':'; } + + /* all good */ + continue; + +lmm_fail: + guc_free(rawstring); + list_free(elemlist); + return false; } /* @@ -1443,7 +1430,7 @@ check_log_min_messages(char **newval, void **extra, GucSource source) */ if (genericlevel == -1) { - GUC_check_errdetail("Generic log level was not defined."); + GUC_check_errdetail("Default log level was not defined."); guc_free(rawstring); list_free(elemlist); return false; @@ -1461,24 +1448,18 @@ check_log_min_messages(char **newval, void **extra, GucSource source) } /* - * Use a stable representation of log_min_messages. The generic level is - * always the first element and the other elements (type:level) are sorted - * by process type. See log_min_messages_cmp for details. + * To present a nice view to users, sort the output list by process type, + * with the default value first. */ list_sort(elemlist, log_min_messages_cmp); - initStringInfo(&buf); + initStringInfoExt(&buf, strlen(rawstring) + 1); foreach_ptr(char, tok, elemlist) { - if (first) - { + if (foreach_current_index(tok) == 0) appendStringInfoString(&buf, tok); - first = false; - } else - { appendStringInfo(&buf, ", %s", tok); - } } result = (char *) guc_malloc(LOG, buf.len + 1); @@ -1505,6 +1486,10 @@ check_log_min_messages(char **newval, void **extra, GucSource source) return true; } +/* + * list_sort() callback for check_log_min_messages. The default element + * goes first; the rest are ordered by strcmp() of the process type. + */ static int log_min_messages_cmp(const ListCell *a, const ListCell *b) { diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index 08553c0f024..05b1feef3cf 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -178,7 +178,7 @@ typedef struct } child_process_kind; static child_process_kind child_process_kinds[] = { -#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \ +#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach) \ [bktype] = {description, main_func, shmem_attach}, #include "postmaster/proctypelist.h" #undef PG_PROCTYPE diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index efba20fa8b9..03f6c8479f2 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -266,7 +266,7 @@ GetBackendTypeDesc(BackendType backendType) switch (backendType) { -#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \ +#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach) \ case bktype: backendDesc = description; break; #include "postmaster/proctypelist.h" #undef PG_PROCTYPE diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 0aebb9c8e24..53933f53b25 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -648,23 +648,15 @@ char *role_string; bool in_hot_standby_guc; /* - * log_min_messages + * set default log_min_messages to WARNING for all process types */ int log_min_messages[] = { -#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \ - [bktype] = log_min_messages, +#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach) \ + [bktype] = WARNING, #include "postmaster/proctypelist.h" #undef PG_PROCTYPE }; -const char *const log_min_messages_process_types[] = { -#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \ - [bktype] = bkcategory, -#include "postmaster/proctypelist.h" -#undef PG_PROCTYPE -}; - - /* * Displayable names for context types (enum GucContext) * diff --git a/src/include/postmaster/proctypelist.h b/src/include/postmaster/proctypelist.h index 0ddd805f0e7..4e259e84c2d 100644 --- a/src/include/postmaster/proctypelist.h +++ b/src/include/postmaster/proctypelist.h @@ -30,22 +30,22 @@ */ -/* bktype, bkcategory, description, main_func, shmem_attach, log_min_messages */ -PG_PROCTYPE(B_ARCHIVER, "archiver", gettext_noop("archiver"), PgArchiverMain, true, WARNING) -PG_PROCTYPE(B_AUTOVAC_LAUNCHER, "autovacuum", gettext_noop("autovacuum launcher"), AutoVacLauncherMain, true, WARNING) -PG_PROCTYPE(B_AUTOVAC_WORKER, "autovacuum", gettext_noop("autovacuum worker"), AutoVacWorkerMain, true, WARNING) -PG_PROCTYPE(B_BACKEND, "backend", gettext_noop("client backend"), BackendMain, true, WARNING) -PG_PROCTYPE(B_BG_WORKER, "bgworker", gettext_noop("background worker"), BackgroundWorkerMain, true, WARNING) -PG_PROCTYPE(B_BG_WRITER, "bgwriter", gettext_noop("background writer"), BackgroundWriterMain, true, WARNING) -PG_PROCTYPE(B_CHECKPOINTER, "checkpointer", gettext_noop("checkpointer"), CheckpointerMain, true, WARNING) -PG_PROCTYPE(B_DEAD_END_BACKEND, "backend", gettext_noop("dead-end client backend"), BackendMain, true, WARNING) -PG_PROCTYPE(B_INVALID, "postmaster", gettext_noop("unrecognized"), NULL, false, WARNING) -PG_PROCTYPE(B_IO_WORKER, "ioworker", gettext_noop("io worker"), IoWorkerMain, true, WARNING) -PG_PROCTYPE(B_LOGGER, "syslogger", gettext_noop("syslogger"), SysLoggerMain, false, WARNING) -PG_PROCTYPE(B_SLOTSYNC_WORKER, "slotsyncworker", gettext_noop("slotsync worker"), ReplSlotSyncWorkerMain, true, WARNING) -PG_PROCTYPE(B_STANDALONE_BACKEND, "backend", gettext_noop("standalone backend"), NULL, false, WARNING) -PG_PROCTYPE(B_STARTUP, "startup", gettext_noop("startup"), StartupProcessMain, true, WARNING) -PG_PROCTYPE(B_WAL_RECEIVER, "walreceiver", gettext_noop("walreceiver"), WalReceiverMain, true, WARNING) -PG_PROCTYPE(B_WAL_SENDER, "walsender", gettext_noop("walsender"), NULL, true, WARNING) -PG_PROCTYPE(B_WAL_SUMMARIZER, "walsummarizer", gettext_noop("walsummarizer"), WalSummarizerMain, true, WARNING) -PG_PROCTYPE(B_WAL_WRITER, "walwriter", gettext_noop("walwriter"), WalWriterMain, true, WARNING) +/* bktype, bkcategory, description, main_func, shmem_attach */ +PG_PROCTYPE(B_ARCHIVER, "archiver", gettext_noop("archiver"), PgArchiverMain, true) +PG_PROCTYPE(B_AUTOVAC_LAUNCHER, "autovacuum", gettext_noop("autovacuum launcher"), AutoVacLauncherMain, true) +PG_PROCTYPE(B_AUTOVAC_WORKER, "autovacuum", gettext_noop("autovacuum worker"), AutoVacWorkerMain, true) +PG_PROCTYPE(B_BACKEND, "backend", gettext_noop("client backend"), BackendMain, true) +PG_PROCTYPE(B_BG_WORKER, "bgworker", gettext_noop("background worker"), BackgroundWorkerMain, true) +PG_PROCTYPE(B_BG_WRITER, "bgwriter", gettext_noop("background writer"), BackgroundWriterMain, true) +PG_PROCTYPE(B_CHECKPOINTER, "checkpointer", gettext_noop("checkpointer"), CheckpointerMain, true) +PG_PROCTYPE(B_DEAD_END_BACKEND, "backend", gettext_noop("dead-end client backend"), BackendMain, true) +PG_PROCTYPE(B_INVALID, "postmaster", gettext_noop("unrecognized"), NULL, false) +PG_PROCTYPE(B_IO_WORKER, "ioworker", gettext_noop("io worker"), IoWorkerMain, true) +PG_PROCTYPE(B_LOGGER, "syslogger", gettext_noop("syslogger"), SysLoggerMain, false) +PG_PROCTYPE(B_SLOTSYNC_WORKER, "slotsyncworker", gettext_noop("slotsync worker"), ReplSlotSyncWorkerMain, true) +PG_PROCTYPE(B_STANDALONE_BACKEND, "backend", gettext_noop("standalone backend"), NULL, false) +PG_PROCTYPE(B_STARTUP, "startup", gettext_noop("startup"), StartupProcessMain, true) +PG_PROCTYPE(B_WAL_RECEIVER, "walreceiver", gettext_noop("walreceiver"), WalReceiverMain, true) +PG_PROCTYPE(B_WAL_SENDER, "walsender", gettext_noop("walsender"), NULL, true) +PG_PROCTYPE(B_WAL_SUMMARIZER, "walsummarizer", gettext_noop("walsummarizer"), WalSummarizerMain, true) +PG_PROCTYPE(B_WAL_WRITER, "walwriter", gettext_noop("walwriter"), WalWriterMain, true) diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 9a6fd503009..616b10f80d6 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -955,19 +955,19 @@ SHOW log_min_messages; SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1'; --fail ERROR: invalid value for parameter "log_min_messages": "checkpointer:debug2, autovacuum:debug1" -DETAIL: Generic log level was not defined. +DETAIL: Default log level was not defined. SET log_min_messages TO 'debug1, backend:error, fatal'; -- fail ERROR: invalid value for parameter "log_min_messages": "debug1, backend:error, fatal" -DETAIL: Generic log level was already assigned. +DETAIL: Redundant specification of default log level. SET log_min_messages TO 'backend:error, debug1, backend:warning'; -- fail ERROR: invalid value for parameter "log_min_messages": "backend:error, debug1, backend:warning" -DETAIL: Process type "backend" was already assigned. +DETAIL: Redundant log level specification for process type "backend". SET log_min_messages TO 'backend:error, foo:fatal, archiver:debug1'; -- fail ERROR: invalid value for parameter "log_min_messages": "backend:error, foo:fatal, archiver:debug1" DETAIL: Unrecognized process type: "foo". SET log_min_messages TO 'backend:error, checkpointer:bar, archiver:debug1'; -- fail ERROR: invalid value for parameter "log_min_messages": "backend:error, checkpointer:bar, archiver:debug1" -DETAIL: Unrecognized log level: "bar". +DETAIL: Unrecognized log level for process type "checkpointer": "bar". SET log_min_messages TO 'backend:error, checkpointer:debug3, fatal, archiver:debug2, autovacuum:debug1, walsender:debug3'; SHOW log_min_messages; log_min_messages -- 2.47.3