On Fri, Feb 16, 2024 at 9:51 PM Andres Freund <and...@anarazel.de> wrote:
>
> Hi,
>
> On 2024-02-16 21:41:41 +0100, Magnus Hagander wrote:
> > > Maybe I am missing something, but why aren't we just getting the value 
> > > from
> > > the leader's entry, instead of copying it?
> >
> > The answer to that would be "because I didn't think of it" :)
>
> :)
>
>
> > Were you thinking of something like the attached?
>
> > @@ -435,6 +438,22 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
> >                               {
> >                                       values[29] = 
> > Int32GetDatum(leader->pid);
> >                                       nulls[29] = false;
> > +
> > +                                     /*
> > +                                      * The authenticated user in a 
> > parallel worker is the same as the one in
> > +                                      * the leader, so look it up there.
> > +                                      */
> > +                                     if (leader->backendId)
> > +                                     {
> > +                                             LocalPgBackendStatus 
> > *leaderstat = pgstat_get_local_beentry_by_backend_id(leader->backendId);
> > +
> > +                                             if 
> > (leaderstat->backendStatus.st_auth_method != uaReject && 
> > leaderstat->backendStatus.st_auth_method != uaImplicitReject)
> > +                                             {
> > +                                                     nulls[31] = nulls[32] 
> > = false;
> > +                                                     values[31] = 
> > CStringGetTextDatum(hba_authname(leaderstat->backendStatus.st_auth_method));
> > +                                                     values[32] = 
> > CStringGetTextDatum(leaderstat->backendStatus.st_auth_identity);
> > +                                             }
> > +                                     }
>
> Mostly, yes.
>
> I only skimmed the patch, but it sure looks to me that we could end up with
> none of the branches setting 31,32, so I think you'd have to make sure to
> handle that case.

That case sets nulls[] for both of them to true I believe? And when
that is set I don't believe we need to set the values themselves.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/


Reply via email to