Re: System username in pg_stat_activity

Hi,

On Fri, Feb 16, 2024 at 08:17:41PM +0100, Magnus Hagander wrote:
> On Fri, Jan 19, 2024 at 7:20 AM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > Hi,
> >
> > On Thu, Jan 18, 2024 at 04:01:33PM +0100, Magnus Hagander wrote:
> > > On Mon, Jan 15, 2024 at 11:17 AM Bertrand Drouvot
> > > > Did you forget to share the new revision (aka v4)? I can only see the
> > > > "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.
> > >
> >
> > Thanks!
> >
> > Just a few comments:
> >
> > 1 ===
> >
> > +       The authentication method used for authenticating the connection, or
> > +       NULL for background processes.
> >
> > What about? "NULL for background processes (except for parallel workers which
> > 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"?

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

> > 2 ===
> >
> > +       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.
> >
> > Same comment about parallel workers.
> >
> > 3 ===
> >
> > +# 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');
> >
> > What do you think about testing the parallel workers cases too? (I'm not 100%
> > sure it's worth the extra complexity though).
> 
> I'm leaning towards not worth that.

Okay, I'm fine with that too.

Regards,

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