On Tue, Jan 24, 2023 at 6:17 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Mon, 23 Jan 2023 17:36:13 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in
> > On Sun, Jan 22, 2023 at 6:12 PM Takamichi Osumi (Fujitsu)
> > <osumi.takamichi@fujitsu.com> wrote:
> > >
> > >
> > > Attached the updated patch v19.
> > Few comments:
> > 2.
> > + if (IsSet(supported_opts, SUBOPT_MIN_APPLY_DELAY) &&
> > + opts->min_apply_delay > 0 && opts->streaming == LOGICALREP_STREAM_PARALLEL)
> > + ereport(ERROR,
> > + errcode(ERRCODE_SYNTAX_ERROR),
> > + errmsg("%s and %s are mutually exclusive options",
> > + "min_apply_delay > 0", "streaming = parallel"));
> > }
> >
> > I think here we should add a comment for the translator as we are
> > doing in some other nearby cases.
>
> IMHO "foo > bar" is not an "option". I think we say "foo and bar are
> mutually exclusive options" but I think don't say "foo = x and bar = y
> are.. options". I wrote a comment as "this should be more like
> human-speaking" and Euler seems having the same feeling for another
> error message.
>
> Concretely I would spell this as "min_apply_delay cannot be enabled
> when parallel streaming mode is enabled" or something.
>
We can change it but the current message seems to be in line with some
nearby messages like "slot_name = NONE and enabled = true are mutually
exclusive options". So, isn't it better to keep this as one in sync
with existing messages?
--
With Regards,
Amit Kapila.