Hi,
On 1/12/23 7:24 PM, Andres Freund wrote:
Hi,
On 2023-01-12 08:38:57 +0100, Drouvot, Bertrand wrote:
On 1/11/23 11:59 PM, Andres Freund wrote:
Now that this patch renames some fields
I don't mind renaming the fields - the prefixes really don't provide anything
useful. But it's not clear why this is related to this patch? You could just
include the f_ prefix in the macro, no?
Right, but the idea is to take the same approach that the one used in
8018ffbf58 (where placing the prefixes in the macro
would have been possible too).
I'm not super happy about that patch tbh.
Probably should remove PgStat_BackendFunctionEntry.
I think that would be a 3rd patch, agree?
Yep.
I now see that PG_STAT_GET_DBENTRY_FLOAT8 already exists, defined the same
way. But the name fields misleading enough that I'd be inclined to rename it?
PG_STAT_GET_FUNCENTRY_FLOAT8_MS looks good by me. Waiting on what we'll decide
for
the existing PG_STAT_GET_DBENTRY_FLOAT8 (so that I can align for the
PG_STAT_GET_FUNCENTRY_FLOAT8).
+1
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
And it looks to me that removing PgStat_BackendFunctionEntry can be done
independently
I'll first look at 1).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
