Re: query_id, pg_stat_activity, extended query protocol

On Tue, Sep 17, 2024 at 05:01:18PM -0500, Sami Imseih wrote:
> > Then, please see attached two lightly-updated patches. 0001 is for a
> > backpatch down to v14. This is yours to force things in the exec and
> > bind messages for all portal types, with the test (placed elsewhere in
> > 14~15 branches). 0002 is for HEAD to add some sanity checks, blowing
> > up the tests of pg_stat_statements if one is not careful with the
> > query ID reporting.
>
> These 2 patches look good to me; except for the slight typo
> In the commit message of 0002. "backpatch" instead of "backpatck".

Yes, I've noticed this one last Friday and fixed the typo in the
commit log after sending the previous patch series.

> That leaves us with considering v5-0002 [1]. I do think this is good
> for overall correctness of the queryId being advertised after a cache
> revalidation, even if users of pg_stat_activity will hardly notice this.
>
> [1] https://www.postgresql.org/message-id/DB325894-3EE3-4B2E-A18C-4B34E7B2F5EC%40gmail.com

Yeah. I need more time to evaluate this one.

Also, please find one of the scripts I have used for the execute/fetch
case, that simply does an INSERT RETURNING with a small fetch size to
create a larger window in pg_stat_activity where we don't report the
query ID. One can run it like that, crude still on point:
# Download a JDBC driver
# Create the table to use.
psql -c 'create table aa (a int);' postgres
CLASSPATH=postgresql-42.7.4.jar java TestReturning.java

Then, while running the script, you would notice that pg_stat_activity
reports NULL for the query ID with the query text while the batch
fetches are processing. I've taken and expanded one of the scripts
you have sent for 1d477a907e63.

I'd like to get to the point where we are able to test that in core
reliably. The sanity checks in the executor paths are a good step
forward but they do nothing for the fetch cases. Perhaps Andrew
Dunstan work to expose libpq's APIs with the perl TAP tests would
help at some point to control the extended protocol queries, but we
are going to need more for the fetch case as there are no hooks that
would help to grab a query ID. A second option I have in mind would
be to set up an injection point that produces a NOTICE if a query ID
is set when we end processing an execute message, then check the
number of NOTICE messages produced as these can be predictible
depending on the number of rows and the fetch size.. This won't fly
far unless we can control the fetch size.
--
Michael