On Sun, Jul 26, 2020 at 01:54:27PM -0500, Justin Pryzby wrote: > + <row> > + <entry><literal>%P</literal></entry> > + <entry>For a parallel worker, this is the Process ID of its > leader > + process. > + </entry> > + <entry>no</entry> > + </row>
Let's be a maximum simple and consistent with surrounding descriptions here and what we have for pg_stat_activity, say: "Process ID of the parallel group leader, if this process is a parallel query worker." > + case 'P': > + if (MyProc) > + { > + PGPROC *leader = MyProc->lockGroupLeader; > + if (leader == NULL || leader->pid == MyProcPid) > + /* padding only */ > + appendStringInfoSpaces(buf, > + padding > 0 ? padding : -padding); > + else if (padding != 0) > + appendStringInfo(buf, "%*d", padding, leader->pid); > + else > + appendStringInfo(buf, "%d", leader->pid); It seems to me we should document here that the check on MyProcPid ensures that this only prints the leader PID only for parallel workers and discards the leader. > + appendStringInfoChar(&buf, ','); > + > + /* leader PID */ > + if (MyProc) > + { > + PGPROC *leader = MyProc->lockGroupLeader; > + if (leader && leader->pid != MyProcPid) > + appendStringInfo(&buf, "%d", leader->pid); > + } > + Same here. Except for those nits, I have tested the patch and things behave as we want (including padding and docs), so this looks good to me. -- Michael
signature.asc
Description: PGP signature