Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

Hello,

On Sun, 2022-01-02 at 13:28 -0800, Andres Freund wrote:
> Hi,
> 
> This fails with an assertion failure:
> https://cirrus-ci.com/task/5567540742062080?logs=cores#L55
> 
> 
Andres, thank you for your test! I've missed it. Fixed in attached
patch v5.

On Wed, 2021-12-22 at 04:25 +0300, Anton A. Melnikov wrote:
> 
> 
> I completely agree that creating a separate view for these new fields
> is
> the most correct thing to do.

Anton,

I've created a new view named pg_stat_statements_aux. But for now both
views are using the same function pg_stat_statements which returns all
fields. It seems reasonable to me - if sampling solution will need all
values it can query the function.

> Also it might be better to use the term 'auxiliary' and use the same 
> approach as for existent similar vars.

Agreed, renamed to auxiliary term.

> So internally it might look something like this:
> 
> double  aux_min_time[PGSS_NUMKIND];
> double  aux_max_time[PGSS_NUMKIND];
> TimestampTz     aux_stats_reset;
> 
> And at the view level:
>    aux_min_plan_time float8,
>    aux_max_plan_time float8,
>    aux_min_exec_time float8,
>    aux_max_exec_time float8,
>    aux_stats_reset timestamp with time zone
> 
> Functions names might be pg_stat_statements_aux() and 
> pg_stat_statements_aux_reset().
> 

But it seems "stats_reset" term is not quite correct in this case. The
"stats_since" field holds the timestamp of hashtable entry, but not the
reset time. The same applies to aux_stats_since - for new statement it
holds its entry time, but in case of reset it will hold the reset time.

"stats_reset" name seems a little bit confusing to me.

Attached patch v5
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company