Re: Split index and table statistics into different types of stats

On Wed, Aug 12, 2026 at 11:52:16AM +0000, Bertrand Drouvot wrote:
> I've a few comments:

Thanks for the review.

> I agree that keeping a single Relation.pgstat_info pointer makes sense.
> That said, IIUC, it does not require both kinds to use the same allocation
> size.
>
> I wonder if we could keep the single pointer design while using a small common
> header in separate table and index pending structures? That would allow
> Relation.pgstat_info to point to the common header while using a different
> pending_size for each kind.

TBH, I find this a bit unattractive because it reduces code clarity.
There is a bit of memory wasted for pending index data for indexes due
to the fact that the union's size is calculated based on its largest
number. It's nothing new, the split just makes cleaner the handling
of the fields, in terms of which stats kind can touch each part.

> The table and index specific functions and macros assume that the relation
> has the expected kind. That assumption existed before this patch, but both
> kinds previously used PgStat_TableStatus, so those accesses still referred to
> fields that existed in the pending object.
>
> With the new union, a wrong call can modify an unrelated member. For example,
> pgstat_count_index_tuples() on a table updates idx.tuples_returned, which
> overlaps the tab.trans pointer.
>
> The current callers look correct, but worth adding assertions for the expected
> kind in those functions and macros?

Hmm. That sounds like a good idea at the end, based on the stats
kind, before touching any relevant field. That enforces the union
policy on a caller basis.

> -typedef struct PgStat_TableXactStatus
> +typedef struct PgStat_RelXactStatus
> {
>
> PgStat_RelationStatus is common to tables and indexes, but PgStat_RelXactStatus
> tracks transactional tuple changes for tables only.
>
> Would keeping PgStat_TableXactStatus be less ambiguous here? It would also
> match add_tabstat_xact_level() and ensure_tabstat_xact_level().

Works for me. Perhaps I was just too much enthusiastic with this part
of the rename. :p

Updated patch attached.
--
Michael