Proposal to sync SET ROLE and pg_stat_activity - Mailing list pgsql-hackers

From Grant Finnemore
Subject Proposal to sync SET ROLE and pg_stat_activity
Date
Msg-id 48B31CAF.4070705@guruhut.com
Whole thread Raw
Responses Re: Proposal to sync SET ROLE and pg_stat_activity  (Euler Taveira de Oliveira <euler@timbira.com>)
List pgsql-hackers
Hi,

In the manual for SET ROLE, it's noted that an invocation of SET ROLE
will leave the session_user unchanged, but will change the current_user.

Invoking pg_stat_activity after the SET ROLE is changed will however
leave the usename unchanged. (Also from the manual we note that a
snapshot is taken at the first call, although in the case of
current_query and others, the field is updated at regular intervals)

SET SESSION AUTHORIZATION behaves similarly, although in that case,
it's documented that both session_user and current_user are changed
to reflect the new user.

An example:-

test=# select current_user, session_user;
  current_user | session_user
--------------+--------------
  grant        | grant
(1 row)

test=# select usename from pg_stat_activity;
  usename
---------
  grant
(1 row)


test=# set session role bob;
SET
test=> select current_user, session_user;
  current_user | session_user
--------------+--------------
  bob          | grant
(1 row)

test=> select usename from pg_stat_activity;
  usename
---------
  grant
(1 row)


I have on occasion used a database pooling scheme that whenever a
connection is retrieved from the pool, either a SET ROLE or SET
SESSION AUTHORIZATION is issued to enable database level access
restrictions. Similarly, when the connection is returned, a RESET
instruction is issued.

IMHO, it would be advantageous to be able to display which
connections are in use by a given user through the pg_stat_activity
view. Looking through the archives, I've found one other request
for this which AFAICS wasn't answered.

   http://archives.postgresql.org/pgsql-bugs/2007-04/msg00035.php

There are two ways in which this could be done. Firstly, we could
alter the current usename field in the view. This would keep the
view definition the same, but would alter the semantics, which could
affect existing clients. Alternatively, we could introduce another
column that would reflect the role name.

I attach a patch that kinda works for the SET SESSION AUTH case, and
will undertake to complete the work should there be some general
support for this proposal.

Comments?

Regards,
Grant Finnemore
Index: src/backend/postmaster/pgstat.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.179
diff -c -r1.179 pgstat.c
*** src/backend/postmaster/pgstat.c    15 Aug 2008 08:37:39 -0000    1.179
--- src/backend/postmaster/pgstat.c    25 Aug 2008 09:08:46 -0000
***************
*** 2264,2269 ****
--- 2264,2294 ----
  }

  /* ----------
+  * pgstat_report_change_authorization()
+  *
+  *      Called from *** to report the changing of session authorization.
+  * ----------
+  */
+ void
+ pgstat_report_change_authorization(void)
+ {
+         volatile PgBackendStatus *beentry = MyBEEntry;
+
+         if (!pgstat_track_activities || !beentry)
+                 return;
+
+         /*
+          * Update my status entry, following the protocol of bumping
+          * st_changecount before and after.  We use a volatile pointer
+          * here to ensure the compiler doesn't try to get cute.
+          */
+         beentry->st_changecount++;
+         beentry->st_userid = GetSessionUserId();
+         beentry->st_changecount++;
+         Assert((beentry->st_changecount & 1) == 0);
+ }
+
+ /* ----------
   * pgstat_report_waiting() -
   *
   *    Called from lock manager to report beginning or end of a lock wait.
Index: src/backend/utils/init/miscinit.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/init/miscinit.c,v
retrieving revision 1.167
diff -c -r1.167 miscinit.c
*** src/backend/utils/init/miscinit.c    27 Mar 2008 17:24:16 -0000    1.167
--- src/backend/utils/init/miscinit.c    25 Aug 2008 09:08:46 -0000
***************
*** 349,354 ****
--- 349,357 ----
      /* We force the effective user IDs to match, too */
      OuterUserId = userid;
      CurrentUserId = userid;
+
+     /* Let the stats subsystem know of the change */
+     pgstat_report_change_authorization();
  }


Index: src/include/pgstat.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/pgstat.h,v
retrieving revision 1.78
diff -c -r1.78 pgstat.h
*** src/include/pgstat.h    15 Aug 2008 08:37:40 -0000    1.78
--- src/include/pgstat.h    25 Aug 2008 09:08:46 -0000
***************
*** 626,631 ****
--- 626,632 ----

  extern void pgstat_report_activity(const char *what);
  extern void pgstat_report_xact_timestamp(TimestampTz tstamp);
+ extern void pgstat_report_change_authorization(void);
  extern void pgstat_report_waiting(bool waiting);
  extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser);


pgsql-hackers by date:

Previous
From: Peter Schuller
Date:
Subject: Re: Implementing cost limit/delays for insert/delete/update/select
Next
From: Tom Lane
Date:
Subject: Re: Implementing cost limit/delays for insert/delete/update/select