Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 20 Mar 2023 16:32:14 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
	"Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Remove nonmeaningful prefixes in PgStat_* fields
Message-ID: <ZBgL/gO/es0D0EeR@paquier.xyz>
References: <9142f62a-a422-145c-bde0-b5bc498a4ada@gmail.com>
 <20230112171252.xucefmthy5l4l2ec@alvherre.pgsql>
 <20230112180733.agmvpugp6ggorade@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="xOEvr2eDcBvHaKZy"
Content-Disposition: inline
In-Reply-To: <20230112180733.agmvpugp6ggorade@awork3.anarazel.de>
Archived-At: 
 <https://www.postgresql.org/message-id/ZBgL/gO/es0D0EeR%40paquier.xyz>
Precedence: bulk


--xOEvr2eDcBvHaKZy
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--xOEvr2eDcBvHaKZy
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmQYC/4ACgkQnvQgOdby
QH2cDA/9HjpuoIyrsbhMxQp9QlZO8Bb7rwoy49ptA2xxyJOKynyqoOPS9UwjY9qh
/3kIBjNV+d2sjYn2iV7CJF6DJg8cQwrV3CC1y9ZYS7NdnD2B2rDSzkiUqQsSNSk6
bTa3+6l8bLdUWSjTP/jm2GYu8K+6GnYITxEnc/j+SVgIuPwYip9vfrRPVnvGiQaZ
eNzkXzLuAyowAbQMw1vrNaWGTCywt9yA0hNnXt0RbQ4oSNdUrzRY5/76Vp/v/Nh0
VzIyXH77lEOaMXPTJtl3AQ18PCMYPVMmjEhgA/wLCMFgzEFn9Uti8rxmHXBQHTM/
UOFkWhR9UN+mmh//sQzfQRy/mfScXwZf6fiuQ2BuI8JD2gVKuNXqmDyzl07akf84
Z0B4MB1SNDgaKHkM5RBEU/HjiWikBsHXN098xv8STv8AroRPzTTbOcS+H1+OsVwd
JvxnrcHMbEjg3ulPQbp4jnFUOv4c9rsbJhkTJnDxjjBYVuTOcM/wfLTajBgn1zb6
Kexic8Mgi578HvymoviEFOGXmw6roKrZGD3s0L+72hJO4CHTTyH3Egk3M8YIinQD
/1PEIbV32Tnif9x4SOsxgznICIhOr6k0tOlNPoRtwD54VDfAFeq4RIBipgCN+heU
rj2N3xYYBsfF8SbhkfJsRhWTD3ZPocGqatMJMJeyUwdnZBUAQKw=
=ZRGX
-----END PGP SIGNATURE-----

--xOEvr2eDcBvHaKZy--