Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: log_min_messages per backend type
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <202512091806.bsugq2l4wz7f@alvherre.pgsql>
Date: Wed, 10 Dec 2025 10:00:36 +0800
Cc: Euler Taveira <euler@eulerto.com>,
 japin <japinli@hotmail.com>,
 Andres Freund <andres@anarazel.de>,
 pgsql-hackers@lists.postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <1130A10B-E7AE-49F3-9E13-2CD69B3F9DFD@gmail.com>
References: <202512091806.bsugq2l4wz7f@alvherre.pgsql>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Archived-At: 
 <https://www.postgresql.org/message-id/1130A10B-E7AE-49F3-9E13-2CD69B3F9DFD%40gmail.com>
Precedence: bulk


> On Dec 10, 2025, at 02:24, Alvaro Herrera <alvherre@alvh.no-ip.org> =
wrote:
>=20
> On 2025-Dec-09, Alvaro Herrera wrote:
>=20
>> So here's your v6 again with those fixes as 0003 -- let's see what CI
>> thinks of this.  I haven't looked at your doc changes yet.
>=20
> This passed CI, so I have marked it as Ready for Committer.  Further
> comments are still welcome, of course, but if there are none, I intend
> to get it committed in a few days.
>=20
>=20
> I'm not really happy with our usage of the translatable description
> field for things such as %b in log_line_prefix or the ps display; I
> think we should use the shorter names there instead.  Otherwise, you =
end
> up with log lines that look something like this (visible in a server
> with %b in log_line_prefix, which the TAP tests as well as pg_regress
> have):
>=20
>  2025-12-08 21:38:04.304 CET autovacuum launcher[2452437] DEBUG: =
autovacuum launcher started
>=20
> where the bit before the PID is marked for translation.  I think it
> should rather be
>=20
>  2025-12-08 21:38:04.304 CET autovacuum[2452437] DEBUG: autovacuum =
launcher started
>=20
> where that name (the same we'll use in log_min_messages) is not
> translated.
>=20
> However, this issue is rather independent of the patch in this thread,
> so I'm going to discuss it in another thread; the name string though =
is
> added by this patch.
>=20
> --=20
> =C3=81lvaro Herrera        Breisgau, Deutschland  =E2=80=94  =
https://www.EnterpriseDB.com/
> "Java is clearly an example of money oriented programming"  (A. =
Stepanov)

Overall the patch is solid, I just have a couple of suggestions:

1 - user experience
```
evantest=3D# show log_min_messages;
  log_min_messages
---------------------
 warning,backend:log
(1 row)
```

Now =E2=80=9Cshow log_min_messages=E2=80=9D prints the raw string the =
user set, in above example, there is not a white-space between the two =
log levels, and =E2=80=9Cshow=E2=80=9D result doesn=E2=80=99t have a =
white-space between the two log levels either. IMO, =E2=80=9CSHOW =
log_min_messages=E2=80=9D should display a stable result, in other =
words, say =E2=80=9Cfatal, backend:log=E2=80=9D and =E2=80=9Cbackend:log, =
fatal=E2=80=9D should show the same result as they are actually meaning =
the same. So, I would suggest normalize the raw string: put the general =
level in the first place and sort others by process type, then SHOW =
returns the normalized string.

2 - refactoring for 0001
```
+		sep =3D strchr(tok, ':');
+		if (sep =3D=3D NULL)
+		{
+			bool		found =3D false;
+
+			/* Reject duplicates for generic log level. */
+			if (genericlevel !=3D -1)
+			{
+				GUC_check_errdetail("Generic log level =
was already assigned.");
+				guc_free(rawstring);
+				list_free(elemlist);
+				return false;
+			}
+
+			/* Is the log level valid? */
+			for (entry =3D server_message_level_options; =
entry && entry->name; entry++)
+			{
+				if (pg_strcasecmp(entry->name, tok) =3D=3D=
 0)
+				{
+					genericlevel =3D entry->val;
+					found =3D true;
+					break;
+				}
+			}
+
+			if (!found)
+			{
+				GUC_check_errdetail("Unrecognized log =
level: \"%s\".", tok);
+				guc_free(rawstring);
+				list_free(elemlist);
+				return false;
+			}
+		}
+		else
+		{
+			char	   *loglevel;
+			char	   *ptype;
+			bool		found =3D false;
+
+			ptype =3D guc_malloc(LOG, (sep - tok) + 1);
+			if (!ptype)
+			{
+				guc_free(rawstring);
+				list_free(elemlist);
+				return false;
+			}
+			memcpy(ptype, tok, sep - tok);
+			ptype[sep - tok] =3D '\0';
+			loglevel =3D sep + 1;
+
+			/* Is the log level valid? */
+			for (entry =3D server_message_level_options; =
entry && entry->name; entry++)
+			{
+				if (pg_strcasecmp(entry->name, loglevel) =
=3D=3D 0)
+				{
+					found =3D true;
+					break;
+				}
+			}
+
+			if (!found)
+			{
+				GUC_check_errdetail("Unrecognized log =
level: \"%s\".", loglevel);
+				guc_free(ptype);
+				guc_free(rawstring);
+				list_free(elemlist);
+				return false;
+			}
```

In the =E2=80=9Cif=E2=80=9D and =E2=80=9Celse=E2=80=9D clauses, there =
are duplicate code to valid log levels. We should refactor the code to =
avoid the duplication. For example, pull up =E2=80=9Cloglevel=E2=80=9D =
to the =E2=80=9Cfor=E2=80=9D loop level, then we can valid it after the =
=E2=80=9Cif-else=E2=80=9D.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/