Re: Re[2]: [PATCH] pg_stat_statements: add last_execution_start column

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)