From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | bharath(dot)rupireddyforpostgres(at)gmail(dot)com, amit(dot)kapila16(at)gmail(dot)com, alvherre(at)alvh(dot)no-ip(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: A recent message added to pg_upgade |
Date: | 2023-11-02 03:32:07 |
Message-ID: | CAHut+PsiCPEjuugjM6pKt17i7pQD+mCZw9xtRHO-JUZvSHu3QA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Nov 2, 2023 at 2:25 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Thu, Nov 2, 2023 at 1:58 PM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> >
> > Thanks you for the comments!
> >
> > At Wed, 1 Nov 2023 18:08:19 +1100, Peter Smith <smithpb2250(at)gmail(dot)com> wrote in
> > > Hi, here are some minor review comments for the v3 patch.
> > >
> > > ======
> > > src/backend/access/transam/xlog.c
> >
> ...
> > > 2.
> >
> > > + GUC_check_errdetail("\"max_slot_wal_keep_size\" must be set to -1
> > > during binary upgrade mode.");
> >
> > > Some of the other GUC_check_errdetail()'s do not include the GUC name
> > > in the translatable message text. Isn't that a preferred style?
> >
> > > SUGGESTION
> > > GUC_check_errdetail("\"%s\" must be set to -1 during binary upgrade mode.",
> > > "max_slot_wal_keep_size");
> >
> > I believe that that style was adopted to minimize translatable
> > messages by consolidting identical ones that only differ in variable
> > names. I see both versions in the tree. I didn't find necessity to
> > adopt this approach for this specific message, especially since I'm
> > skeptical about adding new messages that end with "must be set to -1
> > during binary upgrade mode". (pg_upgrade sets synchronous_commit,
> > fsync and full_page_writes to "off".)
> >
> > However, some unique messages are in this style, so I'm fine with
> > using that style. Revised accordingly.
> >
>
> Checking this patch yesterday prompted me to create a new thread
> questioning the inconsistencies of the "GUC names in messages". In
> that thread, Tom Lane replied and gave some background information [1]
> about the GUC name embedding versus substitution. In hindsight, I
> think your original message was fine as-is, but there seem to be
> examples of every kind of style, so whatever you do would have some
> precedent.
>
> The patch v4 LGTM.
>
To clarify, all the current code LGTM, but the patch is still missing
a guc_hook test case, right?
======
Kind Regards,
Peter Smith.
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Nathan Bossart | 2023-11-02 03:40:06 | Re: Atomic ops for unlogged LSN |
Previous Message | Peter Smith | 2023-11-02 03:25:53 | Re: A recent message added to pg_upgade |