Re: [PATCH] New predefined role pg_manage_extensions

On Thu, 2024-10-31 at 22:47 +0100, Michael Banck wrote:
> Even though there has not been a lot of discussion on this, here is a
> rebased patch.  I have also added it to the upcoming commitfest.

I had a look at the patch.

> --- a/doc/src/sgml/user-manag.sgml
> +++ b/doc/src/sgml/user-manag.sgml
> @@ -669,6 +669,17 @@ GRANT pg_signal_backend TO admin_user;
> </listitem>
> </varlistentry>
>
> + <varlistentry id="predefined-role-pg-manage-extensions" xreflabel="pg_manage_extensions">
> + <term><varname>pg_manage_extensions</varname></term>
> + <listitem>
> + <para>
> + <literal>pg_manage_extensions</literal> allows creating, removing or
> + updating extensions, even if the extensions are untrusted or the user is
> + not the database owner.
> + </para>
> + </listitem>
> + </varlistentry>
> +

The inaccuracy of the database owner has already been mentioned.

Should we say "creating, altering or dropping extensions" to make the connection to
the respective commands obvious?

> --- a/src/backend/commands/extension.c
> +++ b/src/backend/commands/extension.c
> @@ -994,13 +994,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
> ListCell *lc2;
>
> /*
> - * Enforce superuser-ness if appropriate. We postpone these checks until
> - * here so that the control flags are correctly associated with the right
> + * Enforce superuser-ness/membership of the pg_manage_extensions
> + * predefined role if appropriate. We postpone these checks until here
> + * so that the control flags are correctly associated with the right
> * script(s) if they happen to be set in secondary control files.
> */

This is just an attempt to improve the English:

If appropriate, enforce superuser-ness or membership of the predefined role
pg_manage_extensions.

> - : errhint("Must be superuser to create this extension.")));
> + : errhint("Only roles with privileges of the \"%s\" role are allowed to create this extension.", "pg_manage_extensions")));

I don't see the point of breaking out the role name from the message.
We don't do that in other places.
Besides, I think that the mention of the superuser should be retained.
Moreover, I think that commit 8d9978a717 pretty much established that we
should not quote names if they contain underscores.
Perhaps:

errhint("Must be superuser or member of pg_manage_extensions to create this extension.")));

Yours,
Laurenz Albe