Re: query_id, pg_stat_activity, extended query protocol

On Thu, Sep 12, 2024 at 03:58:27PM -0500, Sami Imseih wrote:
> Yes, adding the asserts in execMain.c is better, but there is complications
> there due to the issue you mention. I think the issue you are bumping into
> is when pg_stat_statements.track_utility = on ( default ), the assert in
> ExecutorStart will fail on EXECUTE. I believe it's because ( need to verify )
> pg_stat_statements.c sets the queryId = 0 in the ProcessUtility hook [1].

Yes.

> I am now thinking it's better to Just keep the tests in pg_stat_statements.
> Having test coverage in pg_stat_statements is better than nothing, and
> check-world ( or similar ) will be able to catch such failures.

I have begun things by applying a patch to add new tests in
pg_stat_statements. It is just something that is useful on its own,
and we had nothing of the kind.

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.

I'm planning to look at that again at the beginning of next week.
--
Michael