v11-0002-review.patch

From d8a6a1824a0f93d5418dceb06fc692bb66b62446 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Fri, 6 Feb 2026 10:24:29 +0100
Subject: [PATCH v11 2/3] 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 287816fb504..aeed21b1548 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7127,15 +7127,15 @@ local0.*    /var/log/postgresql
       </term>
       <listitem>
        <para>
-        Controls which <link linkend="runtime-config-severity-levels">message
-        levels</link> are written to the server log.
-        Valid values are a comma-separated list of <literal>type:level</literal>
-        and a single <literal>level</literal>. The list allows it to use
-        different levels per process type. Only the single <literal>level</literal>
-        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
+        <link linkend="runtime-config-severity-levels">message levels</link>
+        are written to the server log.  Valid values are a comma-separated
+        list of zero or more
+        <literal><replaceable>process type</replaceable>:<replaceable>level</replaceable></literal>
+        entries and exactly one mandatory
+        <literal><replaceable>level</replaceable></literal> entry,
+        which is the default for process types not listed.
+        Valid process types are listed in the table below.
         <simplelist type="vert" columns="4">
          <member><literal>archiver</literal></member>
          <member><literal>autovacuum</literal></member>
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 46a5cf13e4a..741fce8dede 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -657,23 +657,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.39.5