Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?

On Wed, Mar 24, 2021 at 05:12:35AM -0300, Alvaro Herrera wrote:
> On 2021-Mar-22, Bruce Momjian wrote:
>
> > diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
> > index e259531f60..9550de0798 100644
> > --- a/src/include/catalog/pg_proc.dat
> > +++ b/src/include/catalog/pg_proc.dat
> > @@ -5249,9 +5249,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,int4}',
> > - 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}',
> > - 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,leader_pid}',
> > + 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,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}',
> > + 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,leader_pid,queryid}',
>
> BTW why do you put the queryid column at the end of the column list
> here? It seems awkward. Can we put it perhaps between state and query?

I thought that it would be better to have it at the end as it can always be
NULL (and will be by default), which I guess was also the reason to have
leader_pid there. I'm all in favor to have queryid near the query, and
while at it leader_pid near the pid.

> > -const char *clean_querytext(const char *query, int *location, int *len);
> > +const char *CleanQuerytext(const char *query, int *location, int *len);
> > JumbleState *JumbleQuery(Query *query, const char *querytext);
>
> I think pushing in more than one commit is a reasonable approach if they
> are well-contained

They should, as I incrementally built on top of the first one. I also just
double checked the patchset and each new commit compiles and passes the
regression tests.

> but if you do that it'd be better to avoid
> introducing a function with one name and renaming it in your next
> commit.

Oops, I apparently messed a fixup when working on it. Bruce, should I take
care of that of do you want to? I think you have some local modifications
already I'd rather not miss some changes.