Subject: Re: System username in pg_stat_activity

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 15 Jan 2024 10:17:34 +0000

Hi,

On Fri, Jan 12, 2024 at 05:16:53PM +0100, Magnus Hagander wrote:
> On Thu, Jan 11, 2024 at 5:55 PM Bertrand Drouvot
> <[email-masked]> wrote:
> >
> > I'm wondering if it would make sense to populate it for parallel workers too.
> > I think it's doable thanks to d951052, but I'm not sure it's worth it (one could
> > join based on the leader_pid though). OTOH that would be consistent with
> > how the SYSTEM_USER behaves with parallel workers (it's populated).
> 
> I guess one could conceptually argue that "authentication happens int
> he leader". But we do populate it with the other user records, and
> it'd be weird if this one was excluded.
> 
> The tricky thing is that pgstat_bestart() is called long before we
> deserialize the data. But from what I can tell it should be safe to
> change it per the attached? That should be AFAICT an extremely short
> window of time longer before we report it, not enough to matter.

Thanks! Yeah, that seems reasonable to me. Also, I think we should remove the
"MyProcPort" test here then (looking at v3):

+       if (MyProcPort && MyClientConnectionInfo.authn_id)
+               strlcpy(lbeentry.st_auth_identity, MyClientConnectionInfo.authn_id, NAMEDATALEN);
+       else
+               MemSet(&lbeentry.st_auth_identity, 0, sizeof(lbeentry.st_auth_identity));

to get the st_auth_identity propagated to the parallel workers.

> >
> > Same remark regarding the parallel workers case +:
> >
> > - Would it be better to use the `name` datatype for auth_identity?
> 
> I've been going back and forth. And I think my conclusion is that it's
> not a postgres identifier, so it shouldn't be. See the earlier
> discussion, and for example that that's what we do for cert names when
> SSL is used.

Yeah, Okay let's keep text then.

> 
> > - what about "Contains the same value as the identity part in <xref linkend="system-user" />"?

Not sure, but looks like you missed this comment?

> >
> > +                       /*
> > +                        * Trust doesn't set_authn_id(), but we still need to store the
> > +                        * auth_method
> > +                        */
> > +                       MyClientConnectionInfo.auth_method = uaTrust;
> >
> > +1, I think it is useful here to provide "trust" and not a NULL value in the
> > context of this patch.
> 
> Yeah, that's probably "independently correct", but actually useful here.

+1

> > +# Users with md5 auth should show both auth method and name in pg_stat_activity
> >
> > what about "show both auth method and identity"?
> 
> Good spot, yeah, I changed it over to identity everywhere else so it
> should be here as well.

Did you forget to share the new revision (aka v4)? I can only see the
"reorder_parallel_worker_bestart.patch" attached.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

---

From: Magnus Hagander <[email-masked]>
Date: Thu, 18 Jan 2024 16:01:33 +0100

> to get the st_auth_identity propagated to the parallel workers.

Yup, I had done that in v4 which as you noted further down, I forgot to post.

> Not sure, but looks like you missed this comment?

I did. Agree with your comment, and updated now.

> "reorder_parallel_worker_bestart.patch" attached.

I did. Here it is, and also including that suggested docs fix as well
as a rebase on current master.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 210c7c0b02..8fa0e5474e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23462,7 +23462,7 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
 
       <row>
        <entry role="func_table_entry"><para role="func_signature">
-        <indexterm>
+        <indexterm id="system-user">
          <primary>system_user</primary>
         </indexterm>
         <function>system_user</function>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 6e74138a69..25eb7a4bf4 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -784,6 +784,28 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>auth_method</structfield> <type>text</type>
+      </para>
+      <para>
+       The authentication method used for authenticating the connection, or
+       NULL for background processes.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>auth_identity</structfield> <type>text</type>
+      </para>
+      <para>
+       The identity (if any) that the user presented during the authentication
+       cycle before they were assigned a database role.  Contains the same
+       value as the identity part in <xref linkend="system-user" />, or NULL
+       for background processes.
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>application_name</structfield> <type>text</type>
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 849a03e4b6..269ab4e586 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1529,6 +1529,9 @@ ParallelWorkerMain(Datum main_arg)
 	/* Attach to the leader's serializable transaction, if SERIALIZABLE. */
 	AttachSerializableXact(fps->serializable_xact_handle);
 
+	/* Report to the stats system that we are started */
+	pgstat_bestart();
+
 	/*
 	 * We've initialized all of our state now; nothing should change
 	 * hereafter.
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e43e36f5ac..b68c382de1 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -873,6 +873,8 @@ CREATE VIEW pg_stat_activity AS
             S.leader_pid,
             S.usesysid,
             U.rolname AS usename,
+            S.auth_method,
+            S.auth_identity,
             S.application_name,
             S.client_addr,
             S.client_hostname,
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 9bbdc4beb0..6621969154 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -625,9 +625,20 @@ ClientAuthentication(Port *port)
 			status = CheckRADIUSAuth(port);
 			break;
 		case uaCert:
-			/* uaCert will be treated as if clientcert=verify-full (uaTrust) */
+
+			/*
+			 * uaCert will be treated as if clientcert=verify-full further
+			 * down
+			 */
+			break;
 		case uaTrust:
 			status = STATUS_OK;
+
+			/*
+			 * Trust doesn't set_authn_id(), but we still need to store the
+			 * auth_method
+			 */
+			MyClientConnectionInfo.auth_method = uaTrust;
 			break;
 	}
 
@@ -645,6 +656,12 @@ ClientAuthentication(Port *port)
 #endif
 	}
 
+	/*
+	 * All auth methods should have either called set_authn_id() or manually
+	 * set the auth_method if they were successful.
+	 */
+	Assert(status != STATUS_OK || MyClientConnectionInfo.auth_method != 0);
+
 	if (Log_connections && status == STATUS_OK &&
 		!MyClientConnectionInfo.authn_id)
 	{
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 1a1050c8da..f52d1e8f49 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -357,6 +357,12 @@ pgstat_bestart(void)
 	else
 		MemSet(&lbeentry.st_clientaddr, 0, sizeof(lbeentry.st_clientaddr));
 
+	lbeentry.st_auth_method = MyClientConnectionInfo.auth_method;
+	if (MyClientConnectionInfo.authn_id)
+		strlcpy(lbeentry.st_auth_identity, MyClientConnectionInfo.authn_id, NAMEDATALEN);
+	else
+		MemSet(&lbeentry.st_auth_identity, 0, sizeof(lbeentry.st_auth_identity));
+
 #ifdef USE_SSL
 	if (MyProcPort && MyProcPort->ssl_in_use)
 	{
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 30a2063505..24490f7ae8 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -304,7 +304,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
 Datum
 pg_stat_get_activity(PG_FUNCTION_ARGS)
 {
-#define PG_STAT_GET_ACTIVITY_COLS	31
+#define PG_STAT_GET_ACTIVITY_COLS	33
 	int			num_backends = pgstat_fetch_stat_numbackends();
 	int			curr_backend;
 	int			pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0);
@@ -617,6 +617,17 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 				nulls[30] = true;
 			else
 				values[30] = UInt64GetDatum(beentry->st_query_id);
+
+			/* Authenticated user */
+			if (beentry->st_auth_method != uaReject && beentry->st_auth_method != uaImplicitReject)
+			{
+				values[31] = CStringGetTextDatum(hba_authname(beentry->st_auth_method));
+				values[32] = CStringGetTextDatum(beentry->st_auth_identity);
+			}
+			else
+			{
+				nulls[31] = nulls[32] = true;
+			}
 		}
 		else
 		{
@@ -646,6 +657,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			nulls[28] = true;
 			nulls[29] = true;
 			nulls[30] = true;
+			nulls[31] = true;
+			nulls[32] = true;
 		}
 
 		tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 1ad3367159..ce090f6864 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -22,6 +22,7 @@
 #include "access/genam.h"
 #include "access/heapam.h"
 #include "access/htup_details.h"
+#include "access/parallel.h"
 #include "access/session.h"
 #include "access/sysattr.h"
 #include "access/tableam.h"
@@ -1235,7 +1236,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 		process_session_preload_libraries();
 
 	/* report this backend in the PgBackendStatus array */
-	if (!bootstrap)
+	if (!bootstrap && !IsParallelWorker())
 		pgstat_bestart();
 
 	/* close the transaction we started above */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 58811a6530..09ba37c50c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5440,9 +5440,9 @@
   proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f',
   proretset => 't', provolatile => 's', proparallel => 'r',
   prorettype => 'record', proargtypes => 'int4',
-  proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,bool,int4,int8}',
-  proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
-  proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,gss_delegation,leader_pid,query_id}',
+  proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,bool,int4,int8,text,text}',
+  proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,gss_delegation,leader_pid,query_id,auth_method,auth_identity}',
   prosrc => 'pg_stat_get_activity' },
 { oid => '8403', descr => 'describe wait events',
   proname => 'pg_get_wait_events', procost => '10', prorows => '250',
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index 7b8a34f64f..cfc45a2224 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -11,6 +11,7 @@
 #define BACKEND_STATUS_H
 
 #include "datatype/timestamp.h"
+#include "libpq/hba.h"
 #include "libpq/pqcomm.h"
 #include "miscadmin.h"			/* for BackendType */
 #include "storage/backendid.h"
@@ -132,6 +133,10 @@ typedef struct PgBackendStatus
 	SockAddr	st_clientaddr;
 	char	   *st_clienthostname;	/* MUST be null-terminated */
 
+	/* Information about the authenticated user */
+	UserAuth	st_auth_method;
+	char		st_auth_identity[NAMEDATALEN];
+
 	/* Information about SSL connection */
 	bool		st_ssl;
 	PgBackendSSLStatus *st_sslstatus;
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 87e180af3d..7d9c409066 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -176,6 +176,22 @@ is($res, 't',
 	"users with trust authentication use SYSTEM_USER = NULL in parallel workers"
 );
 
+# pg_stat_activity should contain trust and empty string for trust auth
+$res = $node->safe_psql(
+	'postgres',
+	"SELECT auth_method, auth_identity='' FROM pg_stat_activity WHERE pid=pg_backend_pid()",
+	connstr => "user=scram_role");
+is($res, 'trust|t', 'Users with trust authentication should show correctly in pg_stat_activity');
+
+# pg_stat_activity should contain NULL for auth of background processes
+# (test is a bit out of place here, but this is where the other pg_stat_activity.auth* tests are)
+$res = $node->safe_psql(
+	'postgres',
+	"SELECT auth_method IS NULL, auth_identity IS NULL FROM pg_stat_activity WHERE backend_type='checkpointer'",
+);
+is($res, 't|t', 'Background processes should show NULL for auth in pg_stat_activity');
+
+
 # Explicitly specifying an empty require_auth (the default) should always
 # succeed.
 $node->connect_ok("user=scram_role require_auth=",
@@ -483,6 +499,13 @@ is($res, 't',
 	"users with md5 authentication use SYSTEM_USER = md5:role in parallel workers"
 );
 
+# Users with md5 auth should show both auth method and identity in pg_stat_activity
+$res = $node->safe_psql(
+	'postgres',
+	"SELECT auth_method, auth_identity FROM pg_stat_activity WHERE pid=pg_backend_pid()",
+	connstr => "user=scram_role");
+is($res, 'md5|scram_role', 'Users with md5 authentication should show correctly in pg_stat_activity');
+
 # Tests for channel binding without SSL.
 # Using the password authentication method; channel binding can't work
 reset_pg_hba($node, 'all', 'all', 'password');
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 55f2e95352..14c23dbd50 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1744,6 +1744,8 @@ pg_stat_activity| SELECT s.datid,
     s.leader_pid,
     s.usesysid,
     u.rolname AS usename,
+    s.auth_method,
+    s.auth_identity,
     s.application_name,
     s.client_addr,
     s.client_hostname,
@@ -1760,7 +1762,7 @@ pg_stat_activity| SELECT s.datid,
     s.query_id,
     s.query,
     s.backend_type
-   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id)
+   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id, auth_method, auth_identity)
      LEFT JOIN pg_database d ON ((s.datid = d.oid)))
      LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
 pg_stat_all_indexes| SELECT c.oid AS relid,
@@ -1880,7 +1882,7 @@ pg_stat_gssapi| SELECT pid,
     gss_princ AS principal,
     gss_enc AS encrypted,
     gss_delegation AS credentials_delegated
-   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id)
+   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id, auth_method, auth_identity)
   WHERE (client_port IS NOT NULL);
 pg_stat_io| SELECT backend_type,
     object,
@@ -2082,7 +2084,7 @@ pg_stat_replication| SELECT s.pid,
     w.sync_priority,
     w.sync_state,
     w.reply_time
-   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id)
+   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id, auth_method, auth_identity)
      JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid)))
      LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
 pg_stat_replication_slots| SELECT s.slot_name,
@@ -2116,7 +2118,7 @@ pg_stat_ssl| SELECT pid,
     ssl_client_dn AS client_dn,
     ssl_client_serial AS client_serial,
     ssl_issuer_dn AS issuer_dn
-   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id)
+   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, gss_delegation, leader_pid, query_id, auth_method, auth_identity)
   WHERE (client_port IS NOT NULL);
 pg_stat_subscription| SELECT su.oid AS subid,
     su.subname,

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 19 Jan 2024 06:20:00 +0000

Hi,

> as a rebase on current master.

Thanks!

Just a few comments:

1 ===

What about? "NULL for background processes (except for parallel workers which
inherit this information from their leader process)"

2 ===

Same comment about parallel workers.

3 ===

What do you think about testing the parallel workers cases too? (I'm not 100%
sure it's worth the extra complexity though).

Apart from those 3, it looks good to me.

Regards,

---

From: Aleksander Alekseev <[email-masked]>
Date: Fri, 19 Jan 2024 14:33:08 +0300

Hi,

> as a rebase on current master.

```
+    lbeentry.st_auth_method = MyClientConnectionInfo.auth_method;
+    if (MyClientConnectionInfo.authn_id)
+        strlcpy(lbeentry.st_auth_identity,
MyClientConnectionInfo.authn_id, NAMEDATALEN);
+    else
+        MemSet(&lbeentry.st_auth_identity, 0,
sizeof(lbeentry.st_auth_identity));
```

I believe using sizeof(lbeentry.st_auth_identity) instead of
NAMEDATALEN is generally considered a better practice.

Calling MemSet for a CString seems to be an overkill. I suggest
setting lbeentry.st_auth_identity[0] to zero.

Except for these nitpicks v4 LGTM.

-- 
Best regards,
Aleksander Alekseev

---

From: Julien Rouhaud <[email-masked]>
Date: Fri, 19 Jan 2024 20:43:05 +0800

Hi,

Should we use pg_mbcliplen() here?  I don't think there's any strong
guarantee that no multibyte character can be used.  I also agree with
the nearby comment about MemSet being overkill.

+       value as the identity part in <xref linkend="system-user" />, or NULL
I was looking at
https://www.postgresql.org/docs/current/auth-username-maps.html and
noticed that this page is switching between system-user and
system-username.  Should we clean that up while at it?

---

From: Magnus Hagander <[email-masked]>
Date: Fri, 16 Feb 2024 20:17:41 +0100

> inherit this information from their leader process)"

Ugh. That doesn't read very well at all to me. Maybe just "NULL for
background processes without a user"?

> sure it's worth the extra complexity though).

I'm leaning towards not worth that.

---

From: Magnus Hagander <[email-masked]>
Date: Fri, 16 Feb 2024 20:23:22 +0100

> NAMEDATALEN is generally considered a better practice.

We use the NAMEDATALEN method in the rest of the function, so I did it
the same way for consistency. I think if we want to change that, we
should change the whole function at once to keep it consistent.

> setting lbeentry.st_auth_identity[0] to zero.

Fair enough. Will make that change.

//Magnus

---

From: Magnus Hagander <[email-masked]>
Date: Fri, 16 Feb 2024 20:39:26 +0100

> the nearby comment about MemSet being overkill.

Hm. Good question. I don't think there is such a guarantee, no. So
something like attached v5?

Also, wouldn't that problem already exist a few lines down for the SSL parts?

> system-username.  Should we clean that up while at it?

Seems like something we should clean up yes, but not as part of this patch.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index cf3de80394..9403df5886 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23891,7 +23891,7 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
 
       <row>
index 5cf9363ac8..51ed656b17 100644
 
+     <row>
 
index 04227a72d1..bb1060fc66 100644

+	/*
index 1a1050c8da..119319e85a 100644
@@ -357,6 +357,17 @@ pgstat_bestart(void)
 
+		int			len = pg_mbcliplen(MyClientConnectionInfo.authn_id, strlen(MyClientConnectionInfo.authn_id), NAMEDATALEN - 1);
+		memcpy(lbeentry.st_auth_identity, MyClientConnectionInfo.authn_id, len);
+		lbeentry.st_auth_identity[len] = '\0';
+		lbeentry.st_auth_identity[0] = '\0';
 
index 7797876d00..91d0e6ccc6 100644
@@ -1237,7 +1238,7 @@ InitPostgres(const char *in_dbname, Oid dboid,

index 9c120fc2b7..8c55765497 100644

 
 
index b7488d760e..f81b4fee7f 100644
@@ -1746,6 +1746,8 @@ pg_stat_activity| SELECT s.datid,
@@ -1762,7 +1764,7 @@ pg_stat_activity| SELECT s.datid,
@@ -1882,7 +1884,7 @@ pg_stat_gssapi| SELECT pid,
@@ -2085,7 +2087,7 @@ pg_stat_replication| SELECT s.pid,
@@ -2119,7 +2121,7 @@ pg_stat_ssl| SELECT pid,

---

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 19 Feb 2024 08:25:14 +0000

Hi,

> background processes without a user"?

Not sure, as I think it could be NULL for background processes that provided
a user in BackgroundWorkerInitializeConnection() too.

> I'm leaning towards not worth that.

Okay, I'm fine with that too.

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 19 Feb 2024 08:55:20 +0000

Hi,

> Seems like something we should clean up yes, but not as part of this patch.

Agree, done in [1].

[1]: https://www.postgresql.org/message-id/ZdMWux1HpIebkEmd%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,