On Mon, Jul 10, 2023 at 2:07 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li <japinli@hotmail.com> wrote in
> >
> > On Sat, 08 Jul 2023 at 12:48, Michael Paquier <michael@paquier.xyz> wrote:
> > > On Fri, Jul 07, 2023 at 07:23:47PM +0800, Japin Li wrote:
> > >> + appendStringInfoString(&errhint, "\"stderr\"");
> > >> +#ifdef HAVE_SYSLOG
> > >> + appendStringInfoString(&errhint, ", \"syslog\"");
> > >> +#endif
> > >> +#ifdef WIN32
> > >> + appendStringInfoString(&errhint, ", \"eventlog\"");
> > >> +#endif
> > >> + appendStringInfoString(&errhint, ", \"csvlog\", and \"jsonlog\"");
> > >
> > > Hmm. Is that OK as a translatable string?
It seems okay to me but needs to be checked.
> >
> >
> > Sorry for the late reply! I'm not sure. How can I know whether it is translatable?
>
> At the very least, we can't generate comma-separated lists
> programatically because punctuation marks vary across languages.
>
> One potential approach could involve defining the message for every
> potential combination, in full length.
Don't we generate a comma-separated list for an error hint of an enum
parameter? For example, to generate the following error hint:
=# alter system set client_min_messages = 'aaa';
ERROR: invalid value for parameter "client_min_messages": "aaa"
HINT: Available values: debug5, debug4, debug3, debug2, debug1, log,
notice, warning, error.
we use the comma-separated generated by config_enum_get_options() and
do ereport() like:
ereport(elevel,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid value for parameter \"%s\": \"%s\"",
name, value),
hintmsg ? errhint("%s", _(hintmsg)) : 0));
IMO log_destination is a string GUC parameter but its value is the
list of enums. So it makes sense to me to add a hint message like what
we do for enum parameters in case where the user mistypes a wrong
value. I'm not sure why the proposed patch needs to quote the usable
values, though. A similar type of GUC parameter is debug_io_direct.
But I'm not sure we need a hint message for it too as it's a developer
option.
> On top of that, consider "csvlog" as an example, -- it
> doesn't work as expected if logging_collector is off. Although this is
> documented, we don't give any warnings at startup. This seems like a
> bigger issue than the unusable keywords. (I don't mean to suggest to
> fix this, as usual.)
Yes, but I think it's a separate problem.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com