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
