Re: documentation fix for SET ROLE

Thanks for reviewing.

On 3/11/21, 6:59 AM, "Laurenz Albe" <laurenz(dot)albe(at)cybertec(dot)at> wrote:
> I have had a look at the patch, and while I agree that this should
> be documented, I am not happy with the patch as it is.
>
> I think we should *not* document that under "server configuration".
> This is confusing and will lead people to think that a role is
> a configuration parameter. But you cannot add
>
> role = myrole
>
> to "postgresql.conf". A role is not a GUC.
>
> I think that the place to document this is
> doc/src/sgml/ref/alter_role.sgml.

I don't think I totally agree that "role" and "session_authorization"
aren't GUCs. They are defined in guc.c, and "role" is referred to as
a GUC in both miscinit.c and variable.c. Plus, they are usable as
configuration parameters in many of the same ways that ordinary GUCs
are (e.g., SET, ALTER ROLE, ALTER DATABASE). It is true that "role"
and "session_authorization" cannot be set in postgresql.conf and ALTER
SYSTEM SET, and I think we should add a note to this effect in the
documentation. However, I don't see the value in duplicating a
paragraph about "role" and "session_authorization" in a number of
statements that already accept a configuration_parameter and point to
the chapter on Server Configuration.

I do agree that that adding these parameters to the Server
Configuration section is a bit confusing. At the very least, the
proposed patch would add them to the Client Connection Defaults
section, but it's still not ideal. This is probably why they've been
left out so far.

> The second hunk of the patch is in the right place:
>
> --- a/doc/src/sgml/ref/set_role.sgml
> +++ b/doc/src/sgml/ref/set_role.sgml
> @@ -53,9 +53,13 @@ RESET ROLE
> </para>
>
> <para>
> - The <literal>NONE</literal> and <literal>RESET</literal> forms reset the current
> - user identifier to be the current session user identifier.
> - These forms can be executed by any user.
> + The <literal>NONE</literal> form resets the current user identifier to the
> + current session user identifier. The <literal>RESET</literal> form resets
> + the current user identifier to the default value. The default value can be
> + the command-line option value, the per-database default setting, or the
> + per-user default setting for the originally authenticated session user, if
> + any such settings exist. Otherwise, the default value will be the current
> + session user identifier. These forms can be executed by any user.
> </para>
> </refsect1>
>
> Perhaps this could be reworded in a simpler fashion, like:
>
> <literal>SET ROLE NONE</literal> sets the user identifier to the current
> session identifier, as returned by the <function>session_user</function>
> function. <literal>RESET ROLE</literal> sets the user identifier to the
> value it had after you connected to the database. This can be different
> from the session identifier if <literal>ALTER DATABASE</literal> or
> <literal>ALTER ROLE</literal> were used to assign a different default role.
>
> (I hope what I wrote is correct.)

I like the simpler text, but I think it is missing a couple of things.
If no session default was set, RESET ROLE will set the role to the
current session user identifier, which can either be what it was when
you first connected or what you SET SESSION AUTHORIZATION to. The
other thing missing is that "role" can be set via the command-line
options, too. Here's an attempt at including those things:

<literal>SET ROLE NONE</literal> sets the current user identifier to the
current session user identifier, as returned by
<function>session_user</function>. <literal>RESET ROLE</literal> sets the
current user identifier to the connection-time setting specified by the
<link linkend="libpq-connect-options">command-line options</link>,
<link linkend="sql-alterrole"><command>ALTER ROLE</command></link>, or
<link linkend="sql-alterdatabase"><command>ALTER DATABASE</command></link>,
if any such settings exist. Otherwise, <literal>RESET ROLE</literal> sets
the current user identifier to the current session user identifier. These
forms can be executed by any user.

I attached a new version of my proposed patch, but I acknowledge that
much of it is still under discussion.

Nathan