Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry

On Mon, Mar 06, 2023 at 08:33:15AM +0100, Drouvot, Bertrand wrote:
> Thanks for having looked at it!

Looking at that, I have a few comments.

+ tabentry = (PgStat_TableStatus *) entry_ref->pending;
+ tablestatus = palloc(sizeof(PgStat_TableStatus));
+ *tablestatus = *tabentry;
+
[...]
+ for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
+ {
+ tablestatus->t_counts.t_tuples_inserted += trans->tuples_inserted;
+ tablestatus->t_counts.t_tuples_updated += trans->tuples_updated;
+ tablestatus->t_counts.t_tuples_deleted += trans->tuples_deleted;
+ }

- if (entry_ref)
- return entry_ref->pending;
- return NULL;
+ return tablestatus;

From what I get with this change, the number of tuples changed by DMLs
have their computations done a bit earlier, meaning that it would make
all the callers of find_tabstat_entry() pay the computation cost.
Still it is not really going to matter, because we will just do the
computation once when looking at any pending changes of
pg_stat_xact_all_tables for each entry. There are 9 callers of
find_tabstat_entry, with 7 being used for pg_stat_xact_all_tables.
How much do we need to care about the remaining two callers
pg_stat_get_xact_blocks_fetched() and pg_stat_get_xact_blocks_hit()?
Could it be a problem if these two also pay the extra computation cost
if a transaction with many subtransactions (aka )needs to look at their
data? These two are used nowhere, they have pg_proc entries and they
are undocumented, so it is hard to say the impact of this change on
them..

Second question: once the data from the subtransactions is copied,
would it be cleaner to set trans to NULL after the data copy is done?

It would feel a bit safer to me to document that find_tabstat_entry()
is currently only used for this xact system view.. The extra
computation could lead to surprises, actually, if this routine is used
outside this context? Perhaps that's OK, but it does not give me a
warm feeling, just to reshape three functions of pgstatfuncs.c with
macros.
--
Michael