On Fri, Aug 8, 2025 at 5:06 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Fri, Aug 08, 2025 at 01:18:39PM +0900, Shinya Kato wrote:
> > The following functions have been modified to return a TIMESTAMP WITH
> > TIME ZONE value indicating when the statistics were reset:
> > - pg_stat_reset()
> > - pg_stat_reset_shared()
> > - pg_stat_reset_single_table_counters()
> > - pg_stat_reset_backend_stats()
> > - pg_stat_reset_single_function_counters()
> > - pg_stat_reset_slru()
> > - pg_stat_reset_replication_slot()
> > - pg_stat_reset_subscription_stats()
> > - pg_stat_clear_snapshot()
> >
> > For pg_stat_reset_backend_stats() and
> > pg_stat_reset_replication_slot(), the functions return the reset
> > timestamp when a valid input is provided. If an invalid input is given
> > (e.g., an invalid backend PID or replication slot name), the functions
> > return NULL.
> > This allows users to easily determine whether the reset operation was
> > successful based on the return value.
>
> I am not sure that this is a good idea overall, but I'm OK if I finish
> outvoted.
Thanks for the review!
> At the exception of pg_stat_reset_backend_stats(), all the functions
> you are listing above are written so as they never fail, so claiming
> that returning a timestamp value for this reason sounds a bit strange
> to me. And we already know this information based on what the stats
> tables already hold for the reset timestamps set when their stats
> kinds callbacks are called.
You are correct that these functions don't fail, so the lack of a
return value is not an issue. However, it would be more convenient,
for instance, when running these functions before and after a test
script, to get an audit trail of the reset time just by checking the
return value, without having to check the pg_stats_* views.
> I can get behind a change for pg_stat_reset_backend_stats() to make it
> return a status, though, as the information depends on some procnum
> lookups which is triggered depending on what the user provides in
> input.
The same applies to pg_stat_reset_replication_slot(). While specifying
a physical replication slot does not raise an error, the operation has
no effect. I think it would be better if this outcome could be
confirmed via the return value.
--
Best regards,
Shinya Kato
NTT OSS Center
