Re: log_min_messages per backend type - Mailing list pgsql-hackers

Hi Euler,

I just reviewed the patch and got a few comments.

> On Nov 6, 2025, at 21:09, Euler Taveira <euler@eulerto.com> wrote:
>
> On Sun, Oct 5, 2025, at 11:18 AM, Euler Taveira wrote:
>> This new patch contains the following changes:
>>
>> - patch was rebased
>> - use commit dbf8cfb4f02
>> - use some GUC memory allocation functions
>> - avoid one memory allocation (suggested by Japin Li)
>> - rename backend type: logger -> syslogger
>> - adjust tests to increase test coverage
>> - improve documentation and comments to reflect the current state
>>
>
> Patch was rebased since commit fce7c73fba4 broke it. No modifications.
>
>
> --
> Euler Taveira
> EDB   https://www.enterprisedb.com/<v5-0001-log_min_messages-per-backend-type.patch>

1 - variable.c
```
+    /* Initialize the array. */
+    memset(newlevel, WARNING, BACKEND_NUM_TYPES * sizeof(int));
```

I think this statement is wrong, because memset() writes bytes but integers, so this statement will set very byte to
WARNING,which should not be the intention. You will need to use a loop to initialize every element of newlevel. 

2 - variable.c
```
+    /* Parse string into list of identifiers. */
+    if (!SplitGUCList(rawstring, ',', &elemlist))
+    {
+        /* syntax error in list */
+        GUC_check_errdetail("List syntax is invalid.");
+        guc_free(rawstring);
+        list_free(elemlist);
+        return false;
+    }
```

Every element of elemlist points to a position of rawstring, so it’s better to list_free(elemlist) first then
guc_free(rawstring).

3 - launch_backend.c
```
 static inline bool
 should_output_to_server(int elevel)
 {
-    return is_log_level_output(elevel, log_min_messages);
+    return is_log_level_output(elevel, log_min_messages[MyBackendType]);
 }
```

Is it possible that when this function is called, MyBackendType has not been initialized? To be safe, maybe we can
checkif MyBackendType is 0 (B_INVALID), then use the generic log_min_message. 

4 - config.sgml
```
+        Valid values are a comma-separated list of <literal>backendtype:level</literal>
+        and a single <literal>level</literal>. The list allows it to use
+        different levels per backend type. Only the single <literal>level</literal>
+        is mandatory (order does not matter) and it is assigned to the backend
+        types that are not specified in the list.
+        Valid <literal>backendtype</literal> values are <literal>archiver</literal>,
+        <literal>autovacuum</literal>, <literal>backend</literal>,
+        <literal>bgworker</literal>, <literal>bgwriter</literal>,
+        <literal>checkpointer</literal>, <literal>ioworker</literal>,
+        <literal>syslogger</literal>, <literal>slotsyncworker</literal>,
+        <literal>startup</literal>, <literal>walreceiver</literal>,
+        <literal>walsender</literal>, <literal>walsummarizer</literal>, and
+        <literal>walwriter</literal>.
+        Valid <literal>level</literal> values are <literal>DEBUG5</literal>,
+        <literal>DEBUG4</literal>, <literal>DEBUG3</literal>, <literal>DEBUG2</literal>,
+        <literal>DEBUG1</literal>, <literal>INFO</literal>, <literal>NOTICE</literal>,
+        <literal>WARNING</literal>, <literal>ERROR</literal>, <literal>LOG</literal>,
+        <literal>FATAL</literal>, and <literal>PANIC</literal>.  Each level includes
+        all the levels that follow it.  The later the level, the fewer messages are sent
+        to the log.  The default is <literal>WARNING</literal> for all backend types.
+        Note that <literal>LOG</literal> has a different rank here than in
```

* “Valid values are …”, here “are” usually mean both are needed, so maybe change “are” to “can be”.
* It says “the single level is mandatory”, then why there is still a default value?

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