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