Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view? - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?
Date
Msg-id 20210401175642.GA18031@momjian.us
Whole thread Raw
In response to Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?  (Julien Rouhaud <rjuju123@gmail.com>)
Responses Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?  (Bruce Momjian <bruce@momjian.us>)
List pgsql-hackers
On Thu, Apr  1, 2021 at 11:30:15PM +0800, Julien Rouhaud wrote:
> On Thu, Apr 01, 2021 at 11:05:24PM +0800, Julien Rouhaud wrote:
> > On Wed, Mar 31, 2021 at 11:18:45AM -0300, Alvaro Herrera wrote:
> > > On 2021-Mar-31, Bruce Momjian wrote:
> > > > 
> > > > I assume it is since Alvaro didn't reply.  I am planning to apply this
> > > > soon.
> > > 
> > > I'm afraid I don't know enough about how parallel query works to make a
> > > good assessment on this being a good approach or not -- and no time at
> > > present to figure it all out.
> > 
> > I'm far from being an expert either, but at the time I wrote it and
> > looking at the code around it probably seemed sensible.  We could directly call
> > pgstat_get_my_queryid() in ExecSerializePlan() rather than passing it from the
> > various callers though, at least there would be a single source for it.
> 
> Here's a v21 that includes the mentioned change.

You are using:

    /* ----------
     * pgstat_get_my_queryid() -
     *
     *    Return current backend's query identifier.
     */
    uint64
    pgstat_get_my_queryid(void)
    {
        if (!MyBEEntry)
            return 0;
    
        return MyBEEntry->st_queryid;
    }

Looking at log_statement:

    /* Log immediately if dictated by log_statement */
    if (check_log_statement(parsetree_list))
    {
        ereport(LOG,
                (errmsg("statement: %s", query_string),
                 errhidestmt(true),
                 errdetail_execute(parsetree_list)));
        was_logged = true;
    }

it uses the global variable query_string.  I wonder if the query hash
should be a global variable too --- this would more clearly match how we
handle top-level info like query_string.  Digging into the stats system
to get top-level info does seem odd.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.




pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Data type correction in pgstat_report_replslot function parameters
Next
From: Bruce Momjian
Date:
Subject: Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?