Обсуждение: documentation fix for SET ROLE
Hi hackers,
I noticed some interesting role behavior that seems to be either a bug
or a miss in the documentation. The documentation for SET ROLE claims
that RESET ROLE resets "the current user identifier to be the current
session user identifier" [0], but this doesn't seem to hold true when
"role" has been set via pg_db_role_setting. Here is an example:
setup:
postgres=# CREATE ROLE test2;
CREATE ROLE
postgres=# CREATE ROLE test1 LOGIN CREATEROLE IN ROLE test2;
CREATE ROLE
postgres=# ALTER ROLE test1 SET ROLE test2;
ALTER ROLE
after logging in as test1:
postgres=> SELECT SESSION_USER, CURRENT_USER;
session_user | current_user
--------------+--------------
test1 | test2
(1 row)
postgres=> RESET ROLE;
RESET
postgres=> SELECT SESSION_USER, CURRENT_USER;
session_user | current_user
--------------+--------------
test1 | test2
(1 row)
I believe this behavior is caused by the "role" getting set at
PGC_S_GLOBAL, which sets the default used by RESET [1]. IMO this just
requires a small documentation fix. Here is my first attempt:
diff --git a/doc/src/sgml/ref/set_role.sgml b/doc/src/sgml/ref/set_role.sgml
index 739f2c5cdf..a69bfeae24 100644
--- a/doc/src/sgml/ref/set_role.sgml
+++ b/doc/src/sgml/ref/set_role.sgml
@@ -54,7 +54,12 @@ RESET ROLE
<para>
The <literal>NONE</literal> and <literal>RESET</literal> forms reset the current
- user identifier to be the current session user identifier.
+ user identifier to the default value. The default value is whatever value it
+ would be if no <command>SET</command> had been executed in the current
+ session. This can be the command-line option value, the per-database default
+ setting, or the per-user default setting for the role, 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>
Nathan
[0] https://www.postgresql.org/docs/devel/sql-set-role.html
[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/utils/guc.h;h=5004ee41;hb=HEAD#l79
On Wednesday, February 17, 2021, Bossart, Nathan <bossartn@amazon.com> wrote:
postgres=# ALTER ROLE test1 SET ROLE test2;
ALTER ROLE
I would not have expected this to work - “role” isn’t a configuration_parameter. Its actually cool that it does, but this doc fix should address this oversight as well.
David J.
On 2/17/21 2:12 PM, David G. Johnston wrote: > On Wednesday, February 17, 2021, Bossart, Nathan <bossartn@amazon.com > <mailto:bossartn@amazon.com>> wrote: > > > postgres=# ALTER ROLE test1 SET ROLE test2; > ALTER ROLE > > > I would not have expected this to work - “role” isn’t a > configuration_parameter. Its actually cool that it does, but this doc fix > should address this oversight as well. I was surprised this worked too. But the behavior is consistent with other GUCs. In other words, when you "ALTER ROLE ... SET ..." you change the default value for the session, and therefore a RESET just changes to that value. -- login as postgres nmx=# show work_mem; work_mem ---------- 200MB (1 row) nmx=# set work_mem = '42MB'; SET nmx=# show work_mem; work_mem ---------- 42MB (1 row) nmx=# reset work_mem; RESET nmx=# show work_mem; work_mem ---------- 200MB (1 row) ALTER ROLE test1 SET work_mem = '42MB'; -- login as test1 nmx=> show work_mem; work_mem ---------- 42MB (1 row) nmx=> reset work_mem; RESET nmx=> show work_mem; work_mem ---------- 42MB (1 row) Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
Вложения
On 2/17/21, 12:15 PM, "Joe Conway" <mail@joeconway.com> wrote:
> On 2/17/21 2:12 PM, David G. Johnston wrote:
>> On Wednesday, February 17, 2021, Bossart, Nathan <bossartn@amazon.com
>> <mailto:bossartn@amazon.com>> wrote:
>>
>>
>> postgres=# ALTER ROLE test1 SET ROLE test2;
>> ALTER ROLE
>>
>>
>> I would not have expected this to work - “role” isn’t a
>> configuration_parameter. Its actually cool that it does, but this doc fix
>> should address this oversight as well.
>
>
> I was surprised this worked too.
>
> But the behavior is consistent with other GUCs. In other words, when you "ALTER
> ROLE ... SET ..." you change the default value for the session, and therefore a
> RESET just changes to that value.
Looking further, I noticed that session_authorization does not work
the same way. AFAICT this is because it's set via SetConfigOption()
in InitializeSessionUserId(). If you initialize role here, it acts
the same as session_authorization.
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 0f67b99cc5..a201bb3766 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -761,6 +761,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
}
/* Record username and superuser status as GUC settings too */
+ SetConfigOption("role", rname, PGC_BACKEND, PGC_S_OVERRIDE);
SetConfigOption("session_authorization", rname,
PGC_BACKEND, PGC_S_OVERRIDE);
SetConfigOption("is_superuser",
Nathan
On 2/17/21 2:12 PM, David G. Johnston wrote: > On Wednesday, February 17, 2021, Bossart, Nathan <bossartn@amazon.com > <mailto:bossartn@amazon.com>> wrote: > > > postgres=# ALTER ROLE test1 SET ROLE test2; > ALTER ROLE > > > I would not have expected this to work - “role” isn’t a > configuration_parameter. Its actually cool that it does, but this doc fix > should address this oversight as well. Here's a patch that adds "role" and "session authorization" as configuration parameters, too. Nathan
Вложения
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".
Yours,
Laurenz Albe
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
On 4/2/21, 10:54 AM, "Joe Conway" <mail@joeconway.com> wrote: > On 4/2/21 10:21 AM, Laurenz Albe wrote: >> So I think that documenting this is the way to go. I'll mark it as >> "ready for committer". > > pushed Thanks! Nathan