On Tue, Oct 01, 2024 at 10:00:00PM +0300, Alexander Lakhin wrote:
> Hello Michael,
>
> 01.10.2024 05:07, Michael Paquier wrote:
> > On Mon, Sep 30, 2024 at 10:07:55AM +0900, Michael Paquier wrote:
> > ...
> > And done this part.
>
> If I'm not missing something, all the patches discussed here are committed
> now, so maybe I've encountered a new anomaly.
>
> Please try the following script:
> BEGIN;
> PREPARE s AS SELECT 1;
> SELECT $1 \bind 1 \g
> EXECUTE s;
>
> It produces for me:
> TRAP: failed Assert("!IsQueryIdEnabled() || !pgstat_track_activities ||
> !debug_query_string || pgstat_get_my_query_id() != 0"), File: "execMain.c",
> Line: 423, PID: 1296466The failure would appear only with pg_stat_statements loaded, not with compute_query_id enabled while pgss is not loaded. The difference is explained by pgss_post_parse_analyze() where the query ID is reset for an ExecuteStmt. The \bind followed by the EXECUTE is at fault here. I thought first that this was some manipulation related to unnamed portals, because PREPARE followed by EXECUTE would assign the ID from the PREPARE in the EXECUTE command when it reaches ExecutorFinish(). However, \bind_named is able to show the same problem, like: SELECT 2 \parse stmt1 begin; PREPARE s AS SELECT 1; \bind_named stmt1 \g EXECUTE s; -- query ID 0 And that's when I noticed that this is only caused by the fact that we would go through ExecuteEnd() and ExecuteFinish() while doing a cleanup of the portal created for \bind[_named]. The query ID is cleaned up first, then the executor end/finish paths are called. This requires also pg_stat_statements.track_utility to be enabled and a transaction block. This proves that the two assertions within ExecutorFinish() and ExecutorEnd() are a bad idea, as they depend on the code paths where an active portal is going to be removed. That leaves the one in ExecutorRun(), lowering the whole value of the check quite a bit. So perhaps it is just better to let that go entirely and finish this experiment? Alexander, I've thought about a couple of fancy cases for ExecutorRun() but I could not break it. Perhaps you have something in your sleeve that would also break this case? -- Michael
signature.asc
Description: PGP signature
