From: | Sami Imseih <samimseih(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, Anthonin Bonnefoy <anthonin(dot)bonnefoy(at)datadoghq(dot)com>, "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, kaido vaikla <kaido(dot)vaikla(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: query_id, pg_stat_activity, extended query protocol |
Date: | 2024-09-12 20:58:27 |
Message-ID: | 6A161F9C-1C2F-450B-9ED7-BB9C809B30FA@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> Do you think that we'd better replace the calls reporting the query ID
> in execMain.c by some assertions on HEAD? This won't work for
> ExecutorStart() because PREPARE statements (or actually EXECUTE,
> e.g. I bumped on that yesterday but I don't recall which one) would
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].
> So your point would be to force this rule within the core executor on
> HEAD? I would not object to that in case we're missing more spots
> with the extended query protocol, actually. That would help us detect
> cases where we're still missing the query ID to be set and the
> executor should know about that.
Yes, but looking at how pg_stat_statements works with PREPARE/EXECUTE,
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 cacth such failures.
> Note that I'm not much worried about the dependency with
> pg_stat_statements. We already rely on it for query jumbling
> normalization for some parse node patterns like DISCARD, and query
> jumbling requires query IDs to be around. So that's not new.
Good point.
Regards,
Sami
From | Date | Subject | |
---|---|---|---|
Next Message | Andreas Karlsson | 2024-09-12 21:09:10 | Re: tiny step toward threading: reduce dependence on setlocale() |
Previous Message | Jeff Davis | 2024-09-12 20:01:45 | Re: tiny step toward threading: reduce dependence on setlocale() |