Обсуждение: 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