Re: missing warning in pg_import_system_collations

Anton Voloshin <a(dot)voloshin(at)postgrespro(dot)ru> writes:
> In pg_import_system_collations() there is this fragment of code:

> enc = pg_get_encoding_from_locale(localebuf, false);
> if (enc < 0)
> {
> /* error message printed by pg_get_encoding_from_locale() */
> continue;
> }

> However, false passed to pg_get_encoding_from_locale() means
> write_message argument is false, so no error message is ever printed.
> I propose an obvious patch (see attachment).
> Introduced in aa17c06fb in January 2017 when debug was replaced by
> false, so I guess back-patching through 10 would be appropriate.

I don't think this is obvious at all.

In the original coding (before aa17c06fb, when this code was in initdb),
we printed a warning if "debug" was true and otherwise printed nothing.
The other "if (debug)" cases in the code that got moved over were
translated to "elog(DEBUG1)", but there isn't any API to make
pg_get_encoding_from_locale() log at that level.

What you propose to do here would promote this case from
ought-to-be-DEBUG1 to WARNING, which seems to me to be way too much in the
user's face. Or, if there actually is a case for complaining, then all
those messages ought to be WARNING not DEBUG1. But I'm inclined to think
that having pg_import_system_collations silently ignore unusable locales
is the right thing most of the time.

Assuming we don't want to change pg_get_encoding_from_locale()'s API,
the simplest fix is to duplicate its error message, so more or less

if (enc < 0)
{
- /* error message printed by pg_get_encoding_from_locale() */
+ elog(DEBUG1, "could not determine encoding for locale \"%s\"",
+ localebuf)));
continue;
}

regards, tom lane