On Fri, Aug 8, 2025 at 5:06 PM Michael Paquier <mich...@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
