Re: move collation import to backend

Hi,

On 2016-10-27 21:56:53 -0400, Peter Eisentraut wrote:
> Currently, initdb parses locale -a output to populate pg_collation. If
> additional collations are installed in the operating system, it is not
> possible to repeat this process, only by doing each step manually. So I
> propose to move this to a backend function that can be called
> separately, and have initdb call that. Running this logic in the
> backend instead of initdb also makes the code simpler. If we add other
> collation providers such as ICU, initdb doesn't need to know about that
> at all, because all the logic would be contained in the backend.

That generally sounds like a good idea. There's some questions imo:
E.g. what if previously present collations are now unavailable?

> I thought about making this a top-level command (IMPORT COLLATIONS ...
> ?) but decided against it for now, to keep it simple.

Seems ok to me.

>
> /*
> * Also forbid matching an any-encoding entry. This test of course is not
> * backed up by the unique index, but it's not a problem since we don't
> * support adding any-encoding entries after initdb.
> */

Note that this isn't true anymore...

> +
> +Datum pg_import_system_collations(PG_FUNCTION_ARGS);
> +
> +Datum
> +pg_import_system_collations(PG_FUNCTION_ARGS)
> +{

Uh?

> + bool if_not_exists = PG_GETARG_BOOL(0);
> + Oid nspid = PG_GETARG_OID(1);
> +
> + FILE *locale_a_handle;
> + char localebuf[NAMEDATALEN]; /* we assume ASCII so this is fine */
> + int count = 0;
> +
> + locale_a_handle = OpenPipeStream("locale -a", "r");
> + if (locale_a_handle == NULL)
> + ereport(ERROR,
> + (errcode_for_file_access(),
> + errmsg("could not execute command \"%s\": %m",
> + "locale -a")));

This function needs to have !superuser permissions revoked, which it
afaics currently hasn't.

Greetings,

Andres Freund