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
