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

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/
>