On Tue, Feb 7, 2023 at 10:07 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Tue, 7 Feb 2023 09:10:01 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in
> > On Tue, Feb 7, 2023 at 6:03 AM Peter Smith <smithpb2250@gmail.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.
--
With Regards,
Amit Kapila.