On Thu, Jan 12, 2023 at 10:07:33AM -0800, Andres Freund wrote: > The problem with that is that the prefixes are used completely inconsistently > - and have been for a long time. And not just between the different type of > stats. Compare e.g. PgStat_TableCounts with PgStat_TableXactStatus and > PgStat_StatTabEntry. Whereas PgStat_FunctionCounts and PgStat_StatFuncEntry > both use it. Right now there's no way to remember where to add the t_ prefix, > and where not. > > Imo the reason to rename here isn't to abolish prefixes, it's to be halfway > consistent within closeby code. And the code overwhelmingly doesn't use the > prefixes.
Reading through the patch, two things are done, basically:
- Remove the prefix "t_" from the fields related to table stats.
- Remove the prefix "f_" from the fields related to function stats.
And FWIW, with my recent lookups at the pgstat code, I'd like to think
that removing the prefixes is actually an improvement in consistency.
It will help in refactoring this code to use more macros, reducing its
size, as well.
So, the code paths where the structures are updated are pretty short
so you know to what they refer to. And that's even more OK because
now the objects are split into their own files, so you know what you
are dealing with even if the individual variable names are more
common. That's for pgstat_relation.c and pgstat_function.c, first.
The second part of the changes involves pgstatfuncs.c, where all the
objects are grouped in a single file. We don't lose any information
here, either, as the code updated deals with a "tabentry" or
"funcentry". There is a small part in pgstat.h where a few macros
have their fields renamed, where we manipulate a "rel", so that looks
rather clear to me what we are dealing with, IMO.
/* Total time previously charged to function, as of function start */
- instr_time save_f_total_time;
+ instr_time save_total_time;
I have something to say about this one, though.. I find this change a
bit confusing. It may be better kept as it is, or I think that we'd
better rename also "save_total" and "start" to reflect in a better way
what they do, because removing "f_" reduces the meaning of the field
with the two others in the same structure.
--
Michael
signature.asc
Description: PGP signature
