Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
JoongHyuk Shin <sjh910805@gmail.com>
From: JoongHyuk Shin <sjh910805@gmail.com>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: Scott Ray <scott@scottray.io>, Fujii Masao <masao.fujii@gmail.com>, Michael Paquier <michael@paquier.xyz>, pgsql-hackers@lists.postgresql.org
Date: 2026-06-21T09:27:42Z
Lists: pgsql-hackers
Attachments
- v5-0001-Don-t-call-ereport-ERROR-from-recovery-target-GUC-as.patch (application/octet-stream) patch v5-0001
Thanks for the reviews. v5 attached. The errdetail now lists which recovery_target_* parameters are actually set, instead of the full candidate list. This follows Scott's idea to surface the set targets; following Álvaro, the values are dropped and a new errhint points to pg_settings for them and their sources. I kept the "which ones are set" list in errdetail rather than errhint, it states the current configuration, which reads as detail, while the errhint carries the actionable pg_settings pointer. -- JH Shin On Mon, Jun 8, 2026 at 12:44 AM Álvaro Herrera <alvherre@kurilemu.de> wrote: > On 2026-Jun-07, JoongHyuk Shin wrote: > > > On "=" vs "set to": I'd stay with "=". > > The list is assembled in appendStringInfo and ends up in the dynamic %s, > > so prose like "set to" there never goes through gettext > > and would print in English even under a translated lc_messages. > > "=" is punctuation, so it sidesteps that. > > Agreed on using =, although ... > > > That said, I'm not sure the currently-set list belongs in the errdetail > at > > all, > > since an operator can read the values back from the configuration. > > I'd be interested in the committer's view on whether it is worth adding. > > It may be enough to list which ones are set, without listing their values. > Those can be obtained easily from pg_settings, which can be mentioned in > errhint -- useful also to figure out exactly _where_ they are set (e.g., > in an include file, postgresql.auto.conf, and so on.) > > -- > Álvaro Herrera Breisgau, Deutschland — > https://www.EnterpriseDB.com/ >