public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: japin <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected]
Subject: Re: log_min_messages per backend type
Date: Wed, 10 Dec 2025 10:00:36 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
> On Dec 10, 2025, at 02:24, Alvaro Herrera <[email protected]> wrote:
>
> On 2025-Dec-09, Alvaro Herrera wrote:
>
>> 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.
>
> 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.
>
>
> 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):
>
> 2025-12-08 21:38:04.304 CET autovacuum launcher[2452437] DEBUG: autovacuum launcher started
>
> where the bit before the PID is marked for translation. I think it
> should rather be
>
> 2025-12-08 21:38:04.304 CET autovacuum[2452437] DEBUG: autovacuum launcher started
>
> where that name (the same we'll use in log_min_messages) is not
> translated.
>
> 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.
>
> --
> Álvaro Herrera Breisgau, Deutschland — 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=# show log_min_messages;
log_min_messages
---------------------
warning,backend:log
(1 row)
```
Now “show log_min_messages” prints the raw string the user set, in above example, there is not a white-space between the two log levels, and “show” result doesn’t have a white-space between the two log levels either. IMO, “SHOW log_min_messages” should display a stable result, in other words, say “fatal, backend:log” and “backend:log, fatal” 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 = strchr(tok, ':');
+ if (sep == NULL)
+ {
+ 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;
+ }
+
+ /* Is the log level valid? */
+ for (entry = server_message_level_options; entry && entry->name; entry++)
+ {
+ if (pg_strcasecmp(entry->name, tok) == 0)
+ {
+ genericlevel = entry->val;
+ found = 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 = false;
+
+ 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';
+ loglevel = sep + 1;
+
+ /* Is the log level valid? */
+ for (entry = server_message_level_options; entry && entry->name; entry++)
+ {
+ if (pg_strcasecmp(entry->name, loglevel) == 0)
+ {
+ found = 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 “if” and “else” clauses, there are duplicate code to valid log levels. We should refactor the code to avoid the duplication. For example, pull up “loglevel” to the “for” loop level, then we can valid it after the “if-else”.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
view thread (20+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: log_min_messages per backend type
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox