Subject: Re: query_id, pg_stat_activity, extended query protocol

From: Michael Paquier <[email-masked]>
Date: Wed, 15 May 2024 14:09:47 +0900

On Wed, May 15, 2024 at 03:24:05AM +0000, Imseih (AWS), Sami wrote:
>> I think we should generally report it when the backend executes a job
>> related to the query with that queryId. This means it would reset the
>> queryId at the end of the query execution.
> 
> When the query completes execution and the session goes into a state 
> other than "active", both the query text and the queryId should be of the 
> last executed statement. This is the documented behavior, and I believe
> it's the correct behavior.
> 
> If we reset queryId at the end of execution, this behavior breaks. Right?

Idle sessions keep track of the last query string run, hence being
consistent in pg_stat_activity and report its query ID is user
friendly.  Resetting it while keeping the string is less consistent.
It's been this way for years, so I'd rather let it be this way.

>> This seems logical, but
>> what about the planning process? If an extension plans a query without
>> the intention to execute it for speculative reasons, should we still
>> show the queryId? Perhaps we should reset the state right after planning
>> to accurately reflect the current queryId.
>
> I think you are suggesting that during planning, the queryId
> of the current statement being planned should not be reported.
>  
> If my understanding is correct, I don't think that is a good idea. Tools that 
> snasphot pg_stat_activity will not be able to account for the queryId during
> planning. This could mean that certain load on the database cannot be tied
> back to a specific queryId.

I'm -1 with the point of resetting the query ID based on what the
patch does, even if it remains available in the hooks.
pg_stat_activity is one thing, but you would also reduce the coverage
of log_line_prefix with %Q.  And that can provide really useful
debugging information in the code paths where the query ID would be
reset as an effect of the proposed patch.

The patch to report the query ID of a planned query when running a
query through a PortalRunSelect() feels more intuitive in the
information it reports.
--
Michael

---

From: Andrei Lepikhov <[email-masked]>
Date: Wed, 15 May 2024 20:29:00 +0700

> It's been this way for years, so I'd rather let it be this way.
Okay, that's what I precisely wanted to understand: queryId doesn't have 
semantics to show the job that consumes resources right now—it is mostly 
about convenience to know that the backend processes nothing except 
(probably) this query.

-- 
regards, Andrei Lepikhov

---

From: "Imseih (AWS), Sami" <[email-masked]>
Date: Wed, 15 May 2024 18:36:23 +0000

> about convenience to know that the backend processes nothing except

It may be a good idea to expose in pg_stat_activity or a
supplemental activity view information about the current state of the
query processing. i.e. Is it parsing, planning or executing a query or
is it processing a nested query. 

I can see this being useful and perhaps could be taken up in a 
separate thread.

Regards,

Sami

---

From: Michael Paquier <[email-masked]>
Date: Thu, 16 May 2024 10:02:54 +0900

> is it processing a nested query. 

pg_stat_activity is already quite bloated with attributes, and I'd
suspect that there are more properties in a query that would be
interesting to track down at a thinner level as long as it mirrors a
dynamic activity of the query.  Perhaps a separate catalog like a
pg_stat_query would make sense, moving query_start there as well?
Catalog breakages are never fun, still always happen because the
reasons behind a backward-incompatible change make the picture better
in the long-term for users.
--
Michael