Re: log_min_messages per backend type

On Thu, Nov 6, 2025, at 11:53 AM, Chao Li wrote:
> I just reviewed the patch and got a few comments.
>

Thanks for your review.

> +	/* 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.
>

Facepalm. Good catch.

> 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).
>

Fixed.

> 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 check if MyBackendType is 0 
> (B_INVALID), then use the generic log_min_message.
>

It uses the generic log level if you don't modify backend type "backend". If
you do specify "backend", that level is used instead. After Alvaro's question
in the other email, I added "postmaster" backend type and assigned it to
B_INVALID. That's exactly the log level that will be used. As I said in that
email, it is ok to consider a process whose backend type is B_INVALID as a
postmaster process.

> * “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?
>

It seems the current sentence that describe the comma-separated list is
ambiguous. The sentence doesn't make it clear that the list contains 2 elements
(type:level and level) and the "level" element is mandatory. What about the new
sentence?


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/