| From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | Alexander Korotkov <aekorotkov(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(at)paquier(dot)xyz>, jian he <jian(dot)universality(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> |
| Subject: | Re: Implement waiting for wal lsn replay: reloaded |
| Date: | 2025-12-30 06:19:27 |
| Message-ID: | CABPTF7Xs-64GQNjmbimZNhj2YSKbBny+evz6=cp3X2fkJS+vMQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, Dec 30, 2025 at 11:25 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Dec 30, 2025, at 11:14, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
> >
> > On 2025-Dec-27, Xuneng Zhou wrote:
> >
> >> On Fri, Dec 26, 2025 at 4:25 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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 format strings, 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 | Content-Type | Size |
|---|---|---|
| v11-0002-Add-MODE-option-to-WAIT-FOR-LSN-command.patch | application/octet-stream | 42.5 KB |
| v11-0001-Extend-xlogwait-infrastructure-with-write-and-fl.patch | application/octet-stream | 11.5 KB |
| v11-0003-Add-tab-completion-for-WAIT-FOR-LSN-MODE-option.patch | application/octet-stream | 2.7 KB |
| v11-0004-Use-WAIT-FOR-LSN-in-PostgreSQL-Test-Cluster-wait.patch | application/octet-stream | 4.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2025-12-30 07:17:26 | Re: Refactor replication origin state reset helpers |
| Previous Message | Pavel Stehule | 2025-12-30 05:32:13 | Re: Can we remove support for standard_conforming_strings = off yet? |