Hi, > >I think we need to add it to track the > >start timestamp in queryDesc. What do you think? > I agree this might be the proper way to go but I don't want to touch > core functionality > in this patch. I'm afraid it could lead to even more problems with > accepting.
> How do you feel about it? I think adding to queryDesc->EState just like we do for other fields that are consumed by pg_stat_statements, i.e. es_processed, es_parallel_workers_to_launch, etc. is the proper pattern. The difference is these other fields have use-cases outside of pg_stat_statements, but I am not sure if that is a reason to deviate from this existing pattern. I also don't think the bounded array in v2 is acceptable. GetCurrentStatementStartTimestamp() does not change for nested levels, it's always the top-level statement start time, so we don't need this. + + /* + * Capture the statement start timestamp now, while it is still + * correct. We cannot rely on reading it later in ExecutorEnd, + * because ExecutorEnd can be deferred until the next Bind message + * in the extended query protocol. + */ + if (nesting_level < PGSS_MAX_NESTING_LEVEL) + pgss_exec_start[nesting_level] = + GetCurrentStatementStartTimestamp(); The same could be accomplished with a single static TimestampTz, but this is still wrong, because what you will need pg_stat_statements to track is an arbitrary number of query executions at any given time, not nesting levels, along with their queryId and toplevel. So, this could get very complex and does not scale well. Whereas just simply tracking the start time in queryDesc->estate will be a much simpler solution. Here is an example with the v2 where it breaks. The case being we have multiple portals open at the same time being accessed as in the case of setting a fetch size. When the portal is closed, the execution start time for both portals is set relative to the second portal. I used JDBC since I can't demo this using psql as \bind always runs the portal to completion. " java DeferredEndTest === Portal A === Time: 2026-04-01 22:13:03.238 A row: id=1 A row: id=2 A row: id=3 --- sleeping 3 seconds --- === Portal B === Time: 2026-04-01 22:13:06.745 B row: id=10000 B row: id=9999 B row: id=9998 === Closing Portal A (ExecutorEnd A) === Time: 2026-04-01 22:13:07.749 === Closing Portal B (ExecutorEnd B) === Time: 2026-04-01 22:13:07.749 === pg_stat_statements results === last_execution_start=2026-04-01 22:13:06.74567 exec_time=1002.44ms query=SELECT id, pg_sleep($1), data FROM big_table ORDER BY id DESC last_execution_start=2026-04-01 22:13:06.74567 exec_time=501.81ms query=SELECT id, pg_sleep($1), data FROM big_table ORDER BY id " If we track the start time in queryDesc->EState, this will not be a problem. For utility statements, we can rely on a direct call to GetCurrentStatementStartTimestamp() We also need to document that for toplevel = false statements, this execution_start time is that of the top level statement. I think that is acceptable. -- Sami Imseih Amazon Web Services (AWS)
DeferredEndTest.java
Description: Binary data
