Hi Euler,
On Thu, 15 Jan 2026 at 13:07, "Euler Taveira" <[email protected]> wrote: > On Tue, Dec 9, 2025, at 3:24 PM, Alvaro Herrera 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 took another look after Chao Li comments [1]. I created the 0003 patch > that does the sort as suggested. I think it is good to be consistent but > I'm fine if we decided the additional code is not worth. The 32 in the > MAX_LMM_STR_LEN is arbitrary but it is based on the size of the largest > element in the list ("dead-end client backend:warning"). I didn't take > into account the comma and space between elements but it is not > necessary since other elements are smaller than the largest one. > I didn't implement the 2nd suggestion. > > I also merged Alvaro's fix to 0002. The v8 is attached. > > I didn't change the commit message but if 0003 is merged into 0001 then > it should mention that > > 8<--------------------------------------------------------------------8< > The SHOW command presents well-formatted list sorted by process type and > the generic log level is the first element list. It improves readability > and has a clear indentation. > 8<--------------------------------------------------------------------8< > > Do we really need a different backend type in this case? For background > workers the description is "background worker". Shoundn't it use the > same description for this edge case too? > > - backend_type_str = MyBgworkerEntry->bgw_type; > + { > + if (MyBgworkerEntry && MyBgworkerEntry->bgw_type[0] != '\0') > + backend_type_str = MyBgworkerEntry->bgw_type; > + else > + backend_type_str = "early bgworker"; > + } > > I also noticed that commit 18d67a8d7d30 forgot to add gettext_noop to > the get_backend_type_for_log function. It should be consistent with > GetBackendTypeDesc() return. > > > [1] https://postgr.es/m/[email protected] > > Thanks for updating the patch set. Here are some comments. 1. We can replace foreach with foreach_ptr in both v8-0001 and v8-0003. 2. +/* log_min_messages */ +extern PGDLLIMPORT const char *const log_min_messages_process_types[]; + Comment looks wrong. 3. For cases where the process type is valid but the log level is unrecognized, I suggest improving the error message for better clarity, e.g.: Unrecognized log level "bar" for process type "backend" 4. The function name string_cmp feels too generic. Could we consider a more specific name, for example list_log_min_message_cmp or another more appropriate one? > -- > Euler Taveira > EDB https://www.enterprisedb.com/ > > [2. text/x-patch; v8-0001-log_min_messages-per-process-type.patch]... > > [3. text/x-patch; v8-0002-Assign-backend-type-earlier.patch]... > > [4. text/x-patch; v8-0003-fixup-log_min_messages-per-process-type.patch]... -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd.
