Thread
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-05-14T05:29:57Z
On Thu, Apr 30, 2026 at 07:38:57PM +0000, Tristan Partin wrote: > Of course I forgot to attach the patch :D. Thanks for the patch. That sounds like a good idea particularly for track_entry_count, because we have no real way to provide this information, which would be more valuable if an in-core stats kind has the idea to switch this flag to true in the future, and I don't really want all extensions to re-implement the same thing to access this information. + if (info->track_entry_count) + { + values[2] = Int64GetDatum(pgstat_get_entry_count(kind)); + } + else + { + nulls[2] = true; + } Returning NULL if the flag is not set sounds sensible. For fixed-numbered, fine by me for 1, so as it is possible to aggregate the full size counting with the fixed shmem size of each stats kind. +SELECT name, builtin FROM pg_stat_kind_info + ORDER BY name COLLATE "C"; This is not going to be stable if running installcheck on an instance where a custom kind is loaded, so let's restrict the query to check for built-in kinds. I'd suggest to expand the data published to more fields and not only what's presented here, so as it becomes possible to do more SQL sanity checks with the stats kind info (same attribute name as the fields): - fixed_amount, where shared_size > 0 does not make sense. - snapshot_ctl_off and shared_ctl_off, that only makes sense under fixed_amount. These should not be set for !fixed_amount. - pending_size, that should not be set for fixed_amount. - existence of flush_pending_cb, delete_pending_cb, reset_timestamp_cb, to_serialized_name, from_serialized_name, to_serialized_data, from_serialized_data (should be booleans), fine for !fixed_amount, never for fixed_amount. - existence of init_shmem_cb, reset_all_cb, snapshot_cb (should be booleans), fine for fixed_amount, never for !fixed_amount. I may be missing one or two things. pgstat_internal.h documents all these requirements, the idea is to translate these requirements at SQL level. We should definitely apply these checks for both custom and built-in stats kinds, which should save time for developers of pgstats in core and outside of core by detecting inconsistent patterns beforehand. -- Michael -
Re: Add pg_stat_kind_info system view
Tristan Partin <tristan@partin.io> — 2026-05-28T17:11:42Z
On Thu May 14, 2026 at 5:30 AM UTC, Michael Paquier wrote: > On Thu, Apr 30, 2026 at 07:38:57PM +0000, Tristan Partin wrote: >> Of course I forgot to attach the patch :D. > > Thanks for the patch. That sounds like a good idea particularly for > track_entry_count, because we have no real way to provide this > information, which would be more valuable if an in-core stats kind has > the idea to switch this flag to true in the future, and I don't really > want all extensions to re-implement the same thing to access this > information. > > + if (info->track_entry_count) > + { > + values[2] = Int64GetDatum(pgstat_get_entry_count(kind)); > + } > + else > + { > + nulls[2] = true; > + } > > Returning NULL if the flag is not set sounds sensible. For > fixed-numbered, fine by me for 1, so as it is possible to aggregate > the full size counting with the fixed shmem size of each stats kind. Awesome. > +SELECT name, builtin FROM pg_stat_kind_info > + ORDER BY name COLLATE "C"; > > This is not going to be stable if running installcheck on an instance > where a custom kind is loaded, so let's restrict the query to check > for built-in kinds. I'll fix this in v2, which I will send after resolving more discussion. I had not considered this as a potential problem. I'll remove the builtin column from the SELECT and filter on builtin as suggested. > I'd suggest to expand the data published to more fields and not only > what's presented here, so as it becomes possible to do more SQL sanity > checks with the stats kind info (same attribute name as the fields): > - fixed_amount, where shared_size > 0 does not make sense. > - snapshot_ctl_off and shared_ctl_off, that only makes sense under > fixed_amount. These should not be set for !fixed_amount. > - pending_size, that should not be set for fixed_amount. > - existence of flush_pending_cb, delete_pending_cb, > reset_timestamp_cb, to_serialized_name, from_serialized_name, > to_serialized_data, from_serialized_data (should be booleans), fine > for !fixed_amount, never for fixed_amount. > - existence of init_shmem_cb, reset_all_cb, snapshot_cb (should be > booleans), fine for fixed_amount, never for !fixed_amount. Can you share how someone might use this additional information? I can see some inherent value for additionally exposing: - fixed_amount - accessed_across_databases - write_to_file - snapshot_ctl_off - shared_ctl_off - shared_data_off - shared_data_len - pending_size > I may be missing one or two things. pgstat_internal.h documents all > these requirements, the idea is to translate these requirements at SQL > level. We should definitely apply these checks for both custom and > built-in stats kinds, which should save time for developers of pgstats > in core and outside of core by detecting inconsistent patterns > beforehand. Not sure I understand why we would want to expose the existence of the callbacks to make assertions at the SQL level. I see that in pgstat_register_kind(), we have the following code: /* * Check some data for fixed-numbered stats. */ if (kind_info->fixed_amount) { if (kind_info->shared_size == 0) ereport(ERROR, (errmsg("custom cumulative statistics property is invalid"), errhint("Custom cumulative statistics require a shared memory size for fixed-numbered objects."))); if (kind_info->track_entry_count) ereport(ERROR, (errmsg("custom cumulative statistics property is invalid"), errhint("Custom cumulative statistics cannot use entry count tracking for fixed-numbered objects."))); } We could extend these invariant checks to make sure that the callbacks are only set when fixed_amount is true for instance. I am very much open to having my mind changed. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)