On Thu, 13 Jul 2023 at 16:19, Masahiko Sawada <sawada.m...@gmail.com> wrote:
> On Tue, Jul 11, 2023 at 10:24 AM Japin Li <japi...@hotmail.com> wrote:
>>
>>
>> On Mon, 10 Jul 2023 at 14:23, Masahiko Sawada <sawada.m...@gmail.com> wrote:
>> > On Mon, Jul 10, 2023 at 2:07 PM Kyotaro Horiguchi
>> > <horikyota....@gmail.com> wrote:
>> >>
>> >> At Mon, 10 Jul 2023 09:04:42 +0800, Japin Li <japi...@hotmail.com> wrote
>> >> in
>> >> >
>> >> > On Sat, 08 Jul 2023 at 12:48, Michael Paquier <mich...@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.
>>
>> I borrowed the description from pg_settings extra_desc. In my first patch,
>> I used the hint message line enum parameter, however, since it might be a
>> combination of multiple log destinations, so, I update the patch using
>> extra_desc.
>
> I agree to use description from pg_settings extra_desc, but it seems
> to be better not to quote each available value like we do for enum
> parameter cases. That is, the hint message would be like:
>
> =# alter system set log_destination to 'xxx';
> ERROR: invalid value for parameter "log_destination": "xxx"
> DETAIL: Unrecognized key word: "xxx".
> HINT: Valid values are combinations of stderr, syslog, csvlog, and jsonlog.
>
Agreed. Fixed as per your suggestions.
--
Regrads,
Japin Li
>From 3f709effe14de81b84bbc441aa5884ece0a757da Mon Sep 17 00:00:00 2001
From: Japin Li <japi...@hotmail.com>
Date: Fri, 14 Jul 2023 09:26:01 +0800
Subject: [PATCH v4 1/1] Add hint message for check_log_destination
---
src/backend/utils/error/elog.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 5898100acb..dccbabf40a 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2228,8 +2228,22 @@ check_log_destination(char **newval, void **extra, GucSource source)
#endif
else
{
+ StringInfoData errhint;
+
+ initStringInfo(&errhint);
+ appendStringInfoString(&errhint, "stderr");
+#ifdef HAVE_SYSLOG
+ appendStringInfoString(&errhint, ", syslog");
+#endif
+#ifdef WIN32
+ appendStringInfoString(&errhint, ", eventlog");
+#endif
+ appendStringInfoString(&errhint, ", csvlog, and jsonlog");
+
GUC_check_errdetail("Unrecognized key word: \"%s\".", tok);
+ GUC_check_errhint("Valid values are combinations of %s.", errhint.data);
pfree(rawstring);
+ pfree(errhint.data);
list_free(elemlist);
return false;
}
--
2.41.0
