Re: documentation fix for SET ROLE - Mailing list pgsql-hackers

On 4/2/21 10:21 AM, Laurenz Albe wrote:
> On Mon, 2021-03-15 at 17:09 +0000, Bossart, Nathan wrote:
>> On 3/15/21, 7:06 AM, "Laurenz Albe" <laurenz.albe@cybertec.at> wrote:
>> > On Fri, 2021-03-12 at 21:41 +0000, Bossart, Nathan wrote:
>> > > On 3/12/21, 11:14 AM, "Joe Conway" <mail@joeconway.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".

pushed

Joe
-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development