Thread

  1. 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)