Re: [Proposal] Adding callback support for custom statistics kinds

On Tue, Dec 09, 2025 at 05:15:45PM -0600, Sami Imseih wrote:
> Thanks for explaining. If there is a good use-case to add more detail to
> the “end” callback, it’s not very obvious yet. Maybe in the future, there
> will be a convincing reason to do so.

The step taken by test_custom_var_stats_file_cleanup() for the
STATS_READ case shines for its simplicity. The STATS_DISCARD case is
also simple: we know that we want to ditch the stats.

Now, it is kind of true that the STATS_WRITE case feels a bit
disturbing written this way: we let a module take an action, but we
don't actually know the state of the main pgstats file when inside the
callback. I mean, you can know how things are going on, but it means
that a module can just rely on a check if
PGSTAT_STAT_PERMANENT_FILENAME is on disk, but an unlink() could have
failed as well. So, yes, I am wondering whether we should do what
Chao is suggesting, passing an extra state to the callback to let the
module know if we have actually succeeded or failed the operations
that have been taken on the main stats file before the callback
end_extra_stats is called in the three cases. It does not matter for
the STATS_READ case, but it may matter for the STATS_DISCARD or
STATS_WRITE case.

> When we hit the clean-up code on any “error”, it should be accompanied by
> an error log. That is
> done in all cases inside pgstat.c, and I expect an extension to log the
> error as well.

FWIW, I still have the same question as the one posted here about the
business in pgstat_initialize(), still present in v6:
https://www.postgresql.org/message-id/aTepXZ97PsXpuywI@paquier.xyz

This remains unanswered.
--
Michael