Thread
-
Re: Re[2]: [PATCH] Add last_executed timestamp to pg_stat_statements
Sami Imseih <samimseih@gmail.com> — 2025-12-11T16:35:41Z
> >Can pg_stat_statements.stats_since help here? > > > >for example "where stats_since > last_poll_timestamp" ? > > Actually no, monitoring tools fetch snapshots to find the difference > between snapshots. > Data for every statement is changes after each execution. > > But stats_since is inserted only once when the new statement execution > appears and is never updated during next executions. I was thinking of using stats_since to avoid fetching query text, since that does not change. But you are talking about avoiding all the stats if they have not changed. I see that now. FWIW, this was discussed back in 2017 [0], and at that time there was some support for last_executed, but the patch did not go anywhere. After looking at the patch, I have a few comments: 1/ There are whitespace errors when applying. 2/ Calling GetCurrentTimestamp while holding a spinlock is not a good idea and should be avoided. This was also a point raised in [0]. Even when we move pg_stat_statements to cumulative stats and not at the mercy of the spinlock for updating entries, i would still hesitate to add an additional GetCurrentTimestamp() for every call. I wonder if we can use GetCurrentStatementStartTimestamp() instead? ``` /* * GetCurrentStatementStartTimestamp */ TimestampTz GetCurrentStatementStartTimestamp(void) { return stmtStartTimestamp; } ``` stmtStartTimestamp is the time the query started, which seems OK for the use-case you are mentioning. But also, stmtStartTimestamp gets set at the top-level so nested entries (toplevel = false ) will just inherit the timestamp of the top-level entry. IMO, this is the most important point in the patch for now. 3/ last_executed, or maybe (last_toplevel_start) if we go with #2 should not be added under pgssEntry->Counters, but rather directory under pgssEntry. @@ -213,6 +214,7 @@ typedef struct Counters * launched */ int64 generic_plan_calls; /* number of calls using a generic plan */ int64 custom_plan_calls; /* number of calls using a custom plan */ + TimestampTz last_executed; /* timestamp of last statement execution */ } Counters; 4/ instead of a " last_executed" maybe the tests should be added to entry_timestamp.sql? [0] https://www.postgresql.org/message-id/flat/CA%2BTgmoZgZMeuN8t9pawSt6M%3DmvxKiAZ4CvPofBWwwVWeZwHe4w%40mail.gmail.com#beeebe3ca4a3dcda4ed625f7c15bb2d8 -- Sami Imseih Amazon Web Services (AWS)