Hi,
On 2023-01-13 10:36:49 +0100, Drouvot, Bertrand wrote:
> > > > Although I suspect this actually hints at an architectural thing that could be
> > > > fixed better: Perhaps we should replace find_tabstat_entry() with a version
> > > > returning a fully "reconciled" PgStat_StatTabEntry? It feels quite wrong to
> > > > have that intimitate knowledge of the subtransaction stuff in pgstatfuncs.c
> > > > and about how the different counts get combined.
> > > >
> > > > I think that'd allow us to move the definition of PgStat_TableStatus to
> > > > PgStat_TableXactStatus, PgStat_TableCounts to pgstat_internal.h. Which feels a
> > > > heck of a lot cleaner.
> > >
> > > Yeah, I think that would be for a 4th patch, agree?
> >
> > Yea. I am of multiple minds about the ordering. I can see benefits on fixing
> > the architectural issue before reducing duplication in the accessor with a
> > macro. The reason is that if we addressed the architectural issue, the
> > difference between the xact and non-xact version will be very minimal, and
> > could even be generated by the same macro.
> >
>
> Yeah, I do agree and I'm in favor of this ordering:
>
> 1) replace find_tabstat_entry() with a version returning a fully "reconciled" PgStat_StatTabEntry
> 2) remove prefixes
> 3) Introduce the new macros
> I'll first look at 1).
Makes sense.
> And it looks to me that removing PgStat_BackendFunctionEntry can be done independently
It's imo the function version of 1), just a bit simpler to implement due to
the much simpler reconciliation. It could be done together with it, or
separately.
Greetings,
Andres Freund