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)
