Re: Time delayed LR (WAS Re: logical replication restrictions)

On Tue, Feb 7, 2023 at 10:42 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Tue, Feb 7, 2023 at 4:02 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Tue, Feb 7, 2023 at 10:07 AM Kyotaro Horiguchi
> > <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > >
> > > At Tue, 7 Feb 2023 09:10:01 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in
> > > > On Tue, Feb 7, 2023 at 6:03 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > > > > 5b.
> > > > > Since there are no translator considerations here why not write the
> > > > > second error like:
> > > > >
> > > > > errmsg("%d ms is outside the valid range for parameter
> > > > > \"min_apply_delay\" (%d .. %d)",
> > > > > result, 0, PG_INT32_MAX))
> > > > >
> > > >
> > > > I see that existing usage in the code matches what the patch had
> > > > before this comment. See below and similar usages in the code.
> > > > if (start <= 0)
> > > > ereport(ERROR,
> > > > (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > > > errmsg("invalid value for parameter \"%s\": %d",
> > > > "start", start)));
> > >
> > > The same errmsg text occurs mamy times in the tree. On the other hand
> > > the pointed message is the only one. I suppose Peter considered this
> > > aspect.
> > >
> > > # "%d%s%s is outside the valid range for parameter \"%s\" (%d .. %d)"
> > > # also appears just once
> > >
> > > As for me, it seems to me a good practice to do that regadless of the
> > > number of duplicates to (semi)mechanically avoid duplicates.
> > >
> > > (But I believe I would do as Peter suggests by myself for the first
> > > cut, though:p)
> > >
> >
> > Personally, I would prefer consistency. I think we can later start a
> > new thread to change the existing message and if there is a consensus
> > and value in the same then we could use the same style here as well.
> >
>
> Of course, if there is a convention then we should stick to it.
>
> My understanding was that (string literal) message parameters are
> specified separately from the message format string primarily as an
> aid to translators. That makes good sense for parameters with names
> that are also English words (like "start" etc), but for non-word
> parameters like "min_apply_delay" there is no such ambiguity in the
> first place.
>

TBH, I am not an expert in this matter. So, to avoid, making any
mistakes I thought of keeping it close to the existing style.

--
With Regards,
Amit Kapila.