Обсуждение: Add stats_reset to pg_stat_all_tables|indexes and related views

Hi hackers,

while working on relfilenode statistics [1], I observed that it's possible to call
pg_stat_reset_single_table_counters() on a relation but the reset time is not
reported in the related views.

I think it's interesting to know when the stats have been reset, so the attached
patch is adding stats_reset to the related views.

Also, it's more consistent with other stat kinds that also report reset times.

This new field is not included into the pg_stat_xact_* views because the 0 values
in transaction-local stats have nothing to do with reset operations.

The patch is pretty straightforward, it:

- Adds a reset_timestamp_cb to PGSTAT_KIND_RELATION
- Adds a function to retrieve the stats_reset field (note that this function is
created on top of the existing PG_STAT_GET_RELENTRY_TIMESTAMPTZ macro, so lacking
some flexibility regarding the function name)
- Adds the stats_reset field in the views that are concerned
- Updates the documentation
- Updates some tests

Regards,

[1]: https://postgr.es/m/ZlGYokUIlERemvpB%40ip-10-97-1-34.eu-west-3.compute.internal

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

> while working on relfilenode statistics [1], I observed that it's possible to call
> pg_stat_reset_single_table_counters() on a relation but the reset time is not
> reported in the related views.
>
> I think it's interesting to know when the stats have been reset, so the attached
> patch is adding stats_reset to the related views.
>
> Also, it's more consistent with other stat kinds that also report reset times.

+1. This field should clearly be there.

> This new field is not included into the pg_stat_xact_* views because the 0 values
> in transaction-local stats have nothing to do with reset operations.
>
> The patch is pretty straightforward, it:

Nothing jumped out at me in the code. Although, I think we should add
at least one test where  pg_stat_reset_single_table_counters() is called
with an index OID. There isn't a difference in the way the stats are
reset for indexes and tables, but they are presented in different views,
so it makes sense to add test coverage.

On a side note: I really think pg_stat_reset_single_table_counters is
the wrong name here, since other OIDs can be used here; indexes
or materialized views, etc. Maybe pg_stat_reset_single_relation_counters
will be better?

--
Sami Imseih
Amazon Web Services (AWS)

On Thu, Oct 02, 2025 at 05:27:06PM -0500, Sami Imseih wrote:
> +1. This field should clearly be there.

Yeah, Bertrand has mentioned this one to me offlist, and I was equally
surprised by the field gone missing.

One question would be if we need to worry about the additional bytes
of this field, but seeing the size of PgStat_StatTabEntry currently
I'm going to answer "no" to my own question in advance.

> Nothing jumped out at me in the code. Although, I think we should add
> at least one test where  pg_stat_reset_single_table_counters() is called
> with an index OID. There isn't a difference in the way the stats are
> reset for indexes and tables, but they are presented in different views,
> so it makes sense to add test coverage.

Makes sense to me.  This matters in terms of coverage for HEAD,
being outside of the scope of this proposal.

> On a side note: I really think pg_stat_reset_single_table_counters is
> the wrong name here, since other OIDs can be used here; indexes
> or materialized views, etc. Maybe pg_stat_reset_single_relation_counters
> will be better?

It's mostly a historical artifact at this stage, and the function is
documented as being usable for an index or a table.  Using "relation"
would be more consistent, indeed.  I am not sure if it's worth
bothering, though.

What's the point of having tests for two tables?  Shouldn't the one
based on test_last_scan be enough?  The one on pg_shdescription may
actually fail on repeated runs, may it not?  It is a shared catalog.
--
Michael

Hi,

On Fri, Oct 03, 2025 at 10:24:39AM +0900, Michael Paquier wrote:
> On Thu, Oct 02, 2025 at 05:27:06PM -0500, Sami Imseih wrote:
> > +1. This field should clearly be there.
> 

Thank you both for looking at it!

> One question would be if we need to worry about the additional bytes
> of this field, but seeing the size of PgStat_StatTabEntry currently
> I'm going to answer "no" to my own question in advance.

Yeah, I was thinking the same and reached the same conclusion.

> > Nothing jumped out at me in the code. Although, I think we should add
> > at least one test where  pg_stat_reset_single_table_counters() is called
> > with an index OID. There isn't a difference in the way the stats are
> > reset for indexes and tables, but they are presented in different views,
> > so it makes sense to add test coverage.
> 
> Makes sense to me.  This matters in terms of coverage for HEAD,
> being outside of the scope of this proposal.

Added one test on pg_stat_all_indexes in v2 attached. That's the first test
on "pg_stat_all_indexes" in .sql files. It just tests the new stats_reset field,
I think it's sufficient for the purpose of this patch.

> > On a side note: I really think pg_stat_reset_single_table_counters is
> > the wrong name here, since other OIDs can be used here; indexes
> > or materialized views, etc. Maybe pg_stat_reset_single_relation_counters
> > will be better?
> 
> It's mostly a historical artifact at this stage,

Yeah, it comes from 083e1b0f27df and the associated discussion is [1]. From what
I can see, at that time the struct that was holding the table and index stats
was "PgStat_TableCounts". So the naming "pg_stat_reset_single_table_counters"
somehow made more sense at that time.

> and the function is
> documented as being usable for an index or a table.  Using "relation"
> would be more consistent, indeed.  I am not sure if it's worth
> bothering, though.

It's done and documented that way since 2010, so I'm also not sure it's worth
bothering.

> What's the point of having tests for two tables?  Shouldn't the one
> based on test_last_scan be enough?  The one on pg_shdescription may
> actually fail on repeated runs, may it not?  It is a shared catalog.

Yeah this one may need to be done differently. I just removed it as it does
not provide extra value here.

[1]: https://www.postgresql.org/message-id/flat/9837222c1001240837r5c103519lc6a74c37be5f1831%40mail.gmail.com

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Fri, Oct 03, 2025 at 05:33:00AM +0000, Bertrand Drouvot wrote:
> On Fri, Oct 03, 2025 at 10:24:39AM +0900, Michael Paquier wrote:
>> Makes sense to me.  This matters in terms of coverage for HEAD,
>> being outside of the scope of this proposal.
>
> Added one test on pg_stat_all_indexes in v2 attached. That's the first test
> on "pg_stat_all_indexes" in .sql files. It just tests the new stats_reset field,
> I think it's sufficient for the purpose of this patch.

But the fact that the reset function is able to work on indexes is
something separate than this patch.  Wouldn't it be better to check
some of the non-timestamp fields of pg_stat_all_indexes with a reset
in a test that applies before the main patch?  That's a minor comment,
sure, but this part feels like a separate item.  So I'd like to
extract that as a patch of its own, then apply it as an independent
piece.

> Yeah, it comes from 083e1b0f27df and the associated discussion is [1]. From what
> I can see, at that time the struct that was holding the table and index stats
> was "PgStat_TableCounts". So the naming "pg_stat_reset_single_table_counters"
> somehow made more sense at that time.

Thanks for the history digging.

I have double-checked the whole patch, at it looks like you have the
right coverage in terms of the docs and the system views, impacting
the set of system views for pg_stat_{all,user,sys}_{tables,indexes},
and same for the pg_statio_* counterparts.  So things look clear here.
--
Michael

Hi,

On Fri, Oct 03, 2025 at 04:50:07PM +0900, Michael Paquier wrote:
> On Fri, Oct 03, 2025 at 05:33:00AM +0000, Bertrand Drouvot wrote:
> > On Fri, Oct 03, 2025 at 10:24:39AM +0900, Michael Paquier wrote:
> >> Makes sense to me.  This matters in terms of coverage for HEAD,
> >> being outside of the scope of this proposal.
> > 
> > Added one test on pg_stat_all_indexes in v2 attached. That's the first test
> > on "pg_stat_all_indexes" in .sql files. It just tests the new stats_reset field,
> > I think it's sufficient for the purpose of this patch.
> 
> But the fact that the reset function is able to work on indexes is
> something separate than this patch.  Wouldn't it be better to check
> some of the non-timestamp fields of pg_stat_all_indexes with a reset
> in a test that applies before the main patch?

Yeah, stats.sql is already doing some tests coverage on index stats (through
retrieving idx_scan and friends in pg_stat_all_tables) but the reset function
is not tested explicitly on indexes.

> That's a minor comment,
> sure, but this part feels like a separate item.  So I'd like to
> extract that as a patch of its own, then apply it as an independent
> piece.

That makes sense. Done in 0001 attached.

> > Yeah, it comes from 083e1b0f27df and the associated discussion is [1]. From what
> > I can see, at that time the struct that was holding the table and index stats
> > was "PgStat_TableCounts". So the naming "pg_stat_reset_single_table_counters"
> > somehow made more sense at that time.
> 
> Thanks for the history digging.
> 
> I have double-checked the whole patch, at it looks like you have the
> right coverage in terms of the docs and the system views, impacting
> the set of system views for pg_stat_{all,user,sys}_{tables,indexes},
> and same for the pg_statio_* counterparts.  So things look clear here.

Thanks! Attached v3, split in 2 sub patches as discussed above (0002 is same
as v2 except that the tests has been changed due to 0001).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com