Re: Implement waiting for wal lsn replay: reloaded - Mailing list pgsql-hackers

From Xuneng Zhou
Subject Re: Implement waiting for wal lsn replay: reloaded
Date
Msg-id CABPTF7Xs-64GQNjmbimZNhj2YSKbBny+evz6=cp3X2fkJS+vMQ@mail.gmail.com
Whole thread Raw
In response to Re: Implement waiting for wal lsn replay: reloaded  (Chao Li <li.evan.chao@gmail.com>)
Responses Re: Implement waiting for wal lsn replay: reloaded
List pgsql-hackers
Hi,

On Tue, Dec 30, 2025 at 11:25 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On Dec 30, 2025, at 11:14, Álvaro Herrera <alvherre@kurilemu.de> wrote:
> >
> > On 2025-Dec-27, Xuneng Zhou wrote:
> >
> >> On Fri, Dec 26, 2025 at 4:25 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> >>> 2 - 0002
> >>> ```
> >>> +                       else
> >>> +                               ereport(ERROR,
> >>> +                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> >>> +                                                errmsg("unrecognized value for WAIT option \"%s\": \"%s\"",
> >>> +                                                               "MODE", mode_str),
> >>> ```
> >>>
> >>> I wonder why don’t we directly put MODE into the error message?
> >>
> >> Yeah, putting MODE into the error message is cleaner. It's done in v8.
> >
> > The reason not to do that (and also put WAIT in a separate string) is so
> > that the message is identicla to other messages and thus requires no
> > separate translation, specifically
> >  errmsg("unrecognized value for %s option \"%s\": \"%s\"", ...)
> >
> > See commit 502e256f2262.  Please use that form.
> >
>
> To follow 502e256f2262, it should use “%s” for “WAIT” as well. I raised the comment because I saw “WAIT” is the
formatstrings, thus “MODE” can be there as well. 
>
> So, we should do a similar change like:
> ```
> -                                                errmsg("unrecognized value for EXPLAIN option \"%s\": \"%s\"",
> -                                                               opt->defname, p),
> +                                                errmsg("unrecognized value for %s option \"%s\": \"%s\"",
> +                                                               "EXPLAIN", opt->defname, p),
> ```
>

Thanks for raising this and clarifying the rationale. I've made the
modification per your input.

--
Best,
Xuneng

Attachment

pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: Can we remove support for standard_conforming_strings = off yet?
Next
From: Chao Li
Date:
Subject: Re: Refactor replication origin state reset helpers