Re: documentation fix for SET ROLE

On Mon, 2021-03-15 at 17:09 +0000, Bossart, Nathan wrote:
> On 3/15/21, 7:06 AM, "Laurenz Albe" <laurenz(dot)albe(at)cybertec(dot)at> wrote:
> > On Fri, 2021-03-12 at 21:41 +0000, Bossart, Nathan wrote:
> > > On 3/12/21, 11:14 AM, "Joe Conway" <mail(at)joeconway(dot)com> wrote:
> > > > Looking back at the commit history it seems to me that this only works
> > > > accidentally. Perhaps it would be best to fix RESET ROLE and be done with it.
> > >
> > > That seems reasonable to me.
> >
> > +1 from me too.
>
> Here's my latest attempt. I think it's important to state that it
> sets the role to the current session user identifier unless there is a
> connection-time setting. If there is no connection-time setting, it
> will reset the role to the current session user, which might be
> different if you've run SET SESSION AUTHORIZATION.
>
> diff --git a/doc/src/sgml/ref/set_role.sgml b/doc/src/sgml/ref/set_role.sgml
> index 739f2c5cdf..f02babf3af 100644
> --- a/doc/src/sgml/ref/set_role.sgml
> +++ b/doc/src/sgml/ref/set_role.sgml
> @@ -53,9 +53,16 @@ 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.
> + <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.
> </para>
> </refsect1>

Actually, SET ROLE NONE is defined by the SQL standard:

18.3 <set role statement>

[...]

If NONE is specified, then
Case:
i) If there is no current user identifier, then an exception condition is raised:
invalid role specification.
ii) Otherwise, the current role name is removed.

This is reflected in a comment in src/backend/commands/variable.c:

/*
* SET ROLE
*
* The SQL spec requires "SET ROLE NONE" to unset the role, so we hardwire
* a translation of "none" to InvalidOid. Otherwise this is much like
* SET SESSION AUTHORIZATION.
*/

On the other hand, RESET (according to src/backend/utils/misc/README)
does something different:

Prior values of configuration variables must be remembered in order to deal
with several special cases: RESET (a/k/a SET TO DEFAULT)

So I think it is intentional that RESET ROLE does something else than
SET ROLE NONE, and we should not change that.

So I think that documenting this is the way to go. I'll mark it as
"ready for committer".

Yours,
Laurenz Albe