public inbox for [email protected]
help / color / mirror / Atom feedFrom: JoongHyuk Shin <[email protected]>
To: Scott Ray <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
Date: Sun, 7 Jun 2026 19:30:02 +0900
Message-ID: <CACSdjfNr8cm1-GPBv8Sqa7N9Uhi9-gR4wHzbTpm9rmW2D=4ftg@mail.gmail.com> (raw)
In-Reply-To: <0WrsV1VojwurDot6hdS0norm0nK9QFDk0KSLqBcXK4Xz2b_sMuIdE0zyiXP0p1hTRD7Hwz3C8AQouGyV7yZMhf5joz3zUpQee3l_pWPs4dk=@scottray.io>
References: <CACSdjfPUa4UvKjADgOERXoxNYmCg2mqqiqKkiJk6mX6E4qgVFw@mail.gmail.com>
<CAHGQGwFNjrLfUSyT4McR4wH_2yFV3n7+63eyvDjzxQchyyQrPA@mail.gmail.com>
<[email protected]>
<CACSdjfPCPjP9tuzxgarcbkjahu-n+o+LJbUQ22aPNkOsOvrEZg@mail.gmail.com>
<CAHGQGwF24XMh=35m+FdqLUw4QgK8HmAGwPZyBDKtsw7MWYp_hw@mail.gmail.com>
<CACSdjfNY=aePEYGYV_5tamz-iTLQLitBuHc8youfsTzfDR1+_g@mail.gmail.com>
<CACSdjfMohHudUh7Fq=pD=Zk+=J=4E65c6VGgScG3CAXvj_Sorg@mail.gmail.com>
<M5a4v6-PJIZSNQlLhGy-GgBPCXNkrW3OnbESqTpaXh2TyZl4TsMyY_wPK-chiMOoZ4mr9Dm7vuqDSVmNeMKfZYNYlI5PxoQyC6iqWZ9MTAk=@scottray.io>
<CACSdjfOxnez=Cv8CzvJcupnaSjaQFnXqk5t=C1ES0BXp52faFg@mail.gmail.com>
<0WrsV1VojwurDot6hdS0norm0nK9QFDk0KSLqBcXK4Xz2b_sMuIdE0zyiXP0p1hTRD7Hwz3C8AQouGyV7yZMhf5joz3zUpQee3l_pWPs4dk=@scottray.io>
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.
PG already uses this form in the RI detail,
e.g. Key (%s)=(%s) in ri_triggers.c.
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.
--
JH Shin
On Sun, Jun 7, 2026 at 5:10 AM Scott Ray <[email protected]> wrote:
> > * Function structure: the recovery_target_* set has been
> > historically stable, so array + loop abstraction adds limited
> > value; function size grows ~34% (32 -> 43 lines) for one line of
> > savings on a hypothetical sixth GUC, while the closest precedent
> > (archive_command / archive_library in pgarch.c) is a hard-coded literal.
> >
>
> > * errhint vs errdetail: errhint("At most one of %s can be set.")
> > reads more like a constraint than an action hint. The closest
> > precedent, archive_command / archive_library in pgarch.c
> > (ProcessPgArchInterrupts() / LoadArchiveLibrary()), keeps the
> > enumeration in errdetail and omits errhint entirely.
> >
>
> > * TAP regex: the added like() uses [^"]+ for the values, which
> > passes regardless of the actual value. Using quotemeta on the
> > expected values would verify the actual content, and anchoring
> > would also avoid accidentally matching the same tokens inside
> > errhint.
>
> Thanks for taking a look. I attached a v2 that applies your suggestions
> and uses "set to" instead of "=" to match convention. What do you think?
>
> Sample output:
>
> FATAL: multiple recovery targets specified
> DETAIL: At most one of "recovery_target", "recovery_target_lsn",
> "recovery_target_name", "recovery_target_time",
> "recovery_target_xid" can be set. Currently set:
> "recovery_target_time" set to "2026-01-01 00:00:00",
> "recovery_target_xid" set to "700".
>
> --
> Scott Ray
>
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
In-Reply-To: <CACSdjfNr8cm1-GPBv8Sqa7N9Uhi9-gR4wHzbTpm9rmW2D=4ftg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox