Feedback-ID: ie3de48e3:Fastmail
Date: Sun, 21 Jun 2026 14:31:55 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: JoongHyuk Shin <sjh910805@gmail.com>
Cc: Scott Ray <scott@scottray.io>, Fujii Masao <masao.fujii@gmail.com>,
	Michael Paquier <michael@paquier.xyz>, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC
 assign hooks
Message-ID: <ajfWZqpzgziKNzX5@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CACSdjfN7uYLS-+wyBbvZ6rmkSNuPm9H8Y+xdd_aSr-kNG_xu3w@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ajfWZqpzgziKNzX5%40alvherre.pgsql>
Precedence: bulk

Hello,

On 2026-Jun-21, JoongHyuk Shin wrote:

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

Please see https://postgr.es/c/3692a622d3fd for more on translatable
message construction.  You should end up with a translatable string in
a _() call like
  _(", \"%s\"")
and the GUC names in a separate string in each case.

Maybe you can make this a local macro to avoid repetitive coding,

#define considerAndComplainAboutGUC(gucname, buf) \
   do { \
       val = GetConfigOption(gucname, false, false); \
       if (val[0] != '\0')     \
       {                       \
            ntargets++;        \
            if (buf.len == 0)  \
                appendStringInfoString(&buf, _("\"%s\""), gucname); \
            else               \
                appendStringInfoString(&buf, _(", \"%s\""), gucname); \
       } \
   } while (0)

considerAndComplainAboutGUC("recovery_target", buf);
considerAndComplainAboutGUC("recovery_target_lsn", buf);
and so on.  (Of course, you should choose a less stupid macro name, but
you get my meaning.)

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"...  In accounting terms this makes perfect sense.  To rational humans, it
is insane.  Welcome to IBM."                           (Robert X. Cringely)
https://www.cringely.com/2015/06/03/autodesks-john-walker-explained-hp-and-ibm-in-1991/