MIME-Version: 1.0
References: <CALDaNm39dtWHoQej2wd74zYGphA0e54Rx6aTRoCEnMXZ5ysbow@mail.gmail.com>
 <CABdArM45=j=DAFTEZA8p=oqYo-XqRwhaPCdKpciQCJx+jNeTJg@mail.gmail.com>
 <TYAPR01MB5692B7687EE7981AA91BA5B9F5362@TYAPR01MB5692.jpnprd01.prod.outlook.com>
 <CABdArM4N60XShbmxXgz4z8ENQGPOMhXSKzvmskdv8V=mQsYOOA@mail.gmail.com>
 <CALDaNm0mTWwg0z4v-sorq08S2CdZmL2s+rh4nHpWeJaBQ2F+mg@mail.gmail.com>
 <CAHut+Psi-mso-qSyMkHRP8e+psDFFmMp4NZTiL_yrFWidHkGMw@mail.gmail.com>
 <CALDaNm3znxKvTv=MDmMBOSk6XurkKgYyh9cH49NLL7SmJ62Q_A@mail.gmail.com>
 <CABdArM7CPR0Ag6OgtufATUn+x=Q_h3cdBcc39r+xeKYDks5Mfw@mail.gmail.com>
 <CALDaNm1FQS04aG0C0gCRpvi-o-OTdq91y6Az34YKN-dVc9r5Ng@mail.gmail.com>
 <CABdArM7LJopsdDyBAhjbsW3K-Yq=f75bnjaU5HRSsTkGWXibaQ@mail.gmail.com>
 <CAHut+PvfSHzKgXJTd=PbJieaXFgR9+VhhR3+eb7jCFFj6kC5bA@mail.gmail.com>
 <CABdArM6qPsZE9v7qymNTwiRt68DgOJ+LsAMhweTxZx1xr1cVOA@mail.gmail.com>
 <CAA4eK1JO+3Q+wkyjhMK+h_5MveTLFY7q2KRbrg1ojtO6dLCaMw@mail.gmail.com>
 <CABdArM4wO2gPfcjrFWWL=D18PyFeWJZJcJGY3uJMzqL+4LDGpw@mail.gmail.com>
 <CAHut+Pt7hYB3kzNgmKJ16z46p9M+4U94k=b=kydPhmZtCtBYHA@mail.gmail.com>
 <CAA4eK1+YyUWkR6WvNzVjgU+QTYtLFo0uofr5d3DKwkLcaOfAsQ@mail.gmail.com>
 <CABdArM6FKY6GjVu=7ak8CAYNK0e329h-JEVa3p0UE5niwP_=yA@mail.gmail.com>
 <CABdArM5oXMO63uETfEFLEan3UfNBiDwprdoyYYdeWP9fsYNg9A@mail.gmail.com>
 <CAHut+PsrSLh3+fSyYWBgHvoPmES1w3RS9xzAQ8_iFLfOqDbF4A@mail.gmail.com>
 <CAA4eK1KT9F6v6CZOZhr9mo8v32yWwYxeNpAc=1aXgpd5KBHtzQ@mail.gmail.com> <CAHut+Pt_jwE51TWOT1R5QP4XbPG-xfYVm_xG6oTopqfVOM2Bxw@mail.gmail.com>
In-Reply-To: <CAHut+Pt_jwE51TWOT1R5QP4XbPG-xfYVm_xG6oTopqfVOM2Bxw@mail.gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Mon, 3 Feb 2025 10:34:35 +0530
Message-ID: <CAA4eK1Jw427qMekbr7KJsAk78o89NP2xJ39tS_A2MHuMb6P=KA@mail.gmail.com>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
To: Peter Smith <smithpb2250@gmail.com>
Cc: Nisha Moond <nisha.moond412@gmail.com>, vignesh C <vignesh21@gmail.com>, 
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, shveta malik <shveta.malik@gmail.com>, 
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Ajin Cherian <itsajin@gmail.com>, 
	Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, 
	Nathan Bossart <nathandbossart@gmail.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAA4eK1Jw427qMekbr7KJsAk78o89NP2xJ39tS_A2MHuMb6P%3DKA%40mail.gmail.com>
Precedence: bulk

On Mon, Feb 3, 2025 at 9:04=E2=80=AFAM Peter Smith <smithpb2250@gmail.com> =
wrote:
>
> On Fri, Jan 31, 2025 at 8:02=E2=80=AFPM Amit Kapila <amit.kapila16@gmail.=
com> wrote:
> >
> > On Fri, Jan 31, 2025 at 10:40=E2=80=AFAM Peter Smith <smithpb2250@gmail=
.com> wrote:
> > >
> > > =3D=3D=3D=3D=3D=3D
> > > src/backend/replication/slot.c
> > >
> > > ReportSlotInvalidation:
> > >
> > > 1.
> > > +
> > > + case RS_INVAL_IDLE_TIMEOUT:
> > > + Assert(inactive_since > 0);
> > > + /* translator: second %s is a GUC variable name */
> > > + appendStringInfo(&err_detail,
> > > + _("The slot has remained idle since %s, which is longer than the
> > > configured \"%s\" duration."),
> > > + timestamptz_to_str(inactive_since),
> > > + "idle_replication_slot_timeout");
> > > + break;
> > > +
> > >
> > > errdetail:
> > >
> > > I guess it is no fault of this patch because I see you've only copied
> > > nearby code, but AFAICT this function is still having an each-way bet
> > > by using a mixture of _() macro which is for strings intended be
> > > translated, but then only using them in errdetail_internal() which is
> > > for strings that are NOT intended to be translated. Isn't it
> > > contradictory? Why don't we use errdetail() here?
> > >
> >
> > Your question is valid and I don't have an answer. I encourage you to
> > start a new thread to clarify this.
> >
>
> I think this was a false alarm.
>
> After studying this more deeply, I've changed my mind and now think
> the code is OK as-is.
>
> AFAICT errdetail_internal is used when not wanting to translate the
> *fmt* string passed to it (see EVALUATE_MESSAGE in elog.c). Now, here
> the format string is just "%s" so it's fine to not translate that.
> Meanwhile, the string value being substituted to the "%s" was already
> translated because of the _(x) macro aka gettext(x).
>

I didn't get your point about " the "%s" was already translated
because of ...". If we don't want to translate the message then why
add '_(' to it in the first place?

--=20
With Regards,
Amit Kapila.