Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 27 Mar 2023 10:20:34 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: "Gregory Stark (as CFM)" <stark.cfm@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	Michael Paquier <michael.paquier@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Generate pg_stat_get_xact*() functions with Macros
Message-ID: <ZCDvYgd8OthCttam@paquier.xyz>
References: <20230111225907.6el6c5j3hukizqxc@awork3.anarazel.de>
 <e6191040-40c4-6849-7b5d-bf3c227f1755@gmail.com>
 <20230112182434.ejffbun4plv2g37k@awork3.anarazel.de>
 <83f7982d-272a-b064-8a0f-8853354edf10@gmail.com>
 <20230113183746.zyncqr2zz3gk6yh6@awork3.anarazel.de>
 <CAM-w4HNq0SmbDa1jCtbmtCzWrx6G7qAGxsjzcOdEyceGOAdJmA@mail.gmail.com>
 <11744b0e-5f7f-aba8-7d9c-2ff0a0c6e2b2@gmail.com>
 <ZBzpDwOMf1ia+M9l@paquier.xyz>
 <631e3084-c5d9-8463-7540-fcff4674caa5@gmail.com>
 <ZB5hipxlAfZoILgZ@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="8SCvqy49/qnuJgUv"
Content-Disposition: inline
In-Reply-To: <ZB5hipxlAfZoILgZ@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/ZCDvYgd8OthCttam%40paquier.xyz>
Precedence: bulk


--8SCvqy49/qnuJgUv
Content-Type: multipart/mixed; boundary="wJjppPOwlPKC1yZ0"
Content-Disposition: inline


--wJjppPOwlPKC1yZ0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 25, 2023 at 11:50:50AM +0900, Michael Paquier wrote:
> On Fri, Mar 24, 2023 at 06:58:30AM +0100, Drouvot, Bertrand wrote:
>> - Does not include the refactoring for
>> pg_stat_get_xact_function_total_time(),
>> pg_stat_get_xact_function_self_time(),=20
>> pg_stat_get_function_total_time() and
>> pg_stat_get_function_self_time(). I think they can be done in a
>> dedicated commit once we agree on the renaming for
>> PG_STAT_GET_DBENTRY_FLOAT8 (see Andres's comment up-thread) so that
>> the new macros can match the future agreement.

Thanks for the reminder.  I have completely missed that this is
mentioned in [1], and that it is all about 8018ffb.  The suggestion to
prefix the macro names with a "_MS" to outline the conversion sounds
like a good one seen from here.  So please find attached a patch to do
this adjustment, completed with a similar switch for the two counters
of the function entries.

>> - Does include the refactoring of the new
>> - pg_stat_get_xact_tuples_newpage_updated() function (added in
>> - ae4fdde135)=20
>=20
> Fine by me.  One step is better than no steps, and this helps:
>  1 file changed, 29 insertions(+), 97 deletions(-)
>=20
> I'll go apply that if there are no objections.

Just did this part to shave a bit more code.

[1]: https://www.postgresql.org/message-id/20230111225907.6el6c5j3hukizqxc@=
awork3.anarazel.de
--
Michael

--wJjppPOwlPKC1yZ0
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="pgstatfuncs-more-macros.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pg=
statfuncs.c
index e1dd1e0ad3..ff155e003f 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -152,29 +152,26 @@ pg_stat_get_function_calls(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(funcentry->numcalls);
 }
=20
-Datum
-pg_stat_get_function_total_time(PG_FUNCTION_ARGS)
-{
-	Oid			funcid =3D PG_GETARG_OID(0);
-	PgStat_StatFuncEntry *funcentry;
-
-	if ((funcentry =3D pgstat_fetch_stat_funcentry(funcid)) =3D=3D NULL)
-		PG_RETURN_NULL();
-	/* convert counter from microsec to millisec for display */
-	PG_RETURN_FLOAT8(((double) funcentry->total_time) / 1000.0);
+/* convert counter from microsec to millisec for display */
+#define PG_STAT_GET_FUNCENTRY_FLOAT8_MS(stat)						\
+Datum																\
+CppConcat(pg_stat_get_function_,stat)(PG_FUNCTION_ARGS)				\
+{																	\
+	Oid			funcid =3D PG_GETARG_OID(0);							\
+	double		result;												\
+	PgStat_StatFuncEntry *funcentry;								\
+																	\
+	if ((funcentry =3D pgstat_fetch_stat_funcentry(funcid)) =3D=3D NULL)	\
+		PG_RETURN_NULL();											\
+	result =3D ((double) funcentry->stat) / 1000.0;					\
+	PG_RETURN_FLOAT8(result);										\
 }
=20
-Datum
-pg_stat_get_function_self_time(PG_FUNCTION_ARGS)
-{
-	Oid			funcid =3D PG_GETARG_OID(0);
-	PgStat_StatFuncEntry *funcentry;
+/* pg_stat_get_function_total_time */
+PG_STAT_GET_FUNCENTRY_FLOAT8_MS(total_time)
=20
-	if ((funcentry =3D pgstat_fetch_stat_funcentry(funcid)) =3D=3D NULL)
-		PG_RETURN_NULL();
-	/* convert counter from microsec to millisec for display */
-	PG_RETURN_FLOAT8(((double) funcentry->self_time) / 1000.0);
-}
+/* pg_stat_get_function_self_time */
+PG_STAT_GET_FUNCENTRY_FLOAT8_MS(self_time)
=20
 Datum
 pg_stat_get_backend_idset(PG_FUNCTION_ARGS)
@@ -1147,7 +1144,8 @@ pg_stat_get_db_checksum_last_failure(PG_FUNCTION_ARGS)
 		PG_RETURN_TIMESTAMPTZ(result);
 }
=20
-#define PG_STAT_GET_DBENTRY_FLOAT8(stat)						\
+/* convert counter from microsec to millisec for display */
+#define PG_STAT_GET_DBENTRY_FLOAT8_MS(stat)						\
 Datum															\
 CppConcat(pg_stat_get_db_,stat)(PG_FUNCTION_ARGS)				\
 {																\
@@ -1164,19 +1162,19 @@ CppConcat(pg_stat_get_db_,stat)(PG_FUNCTION_ARGS)		=
		\
 }
=20
 /* pg_stat_get_db_active_time */
-PG_STAT_GET_DBENTRY_FLOAT8(active_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(active_time)
=20
 /* pg_stat_get_db_blk_read_time */
-PG_STAT_GET_DBENTRY_FLOAT8(blk_read_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(blk_read_time)
=20
 /* pg_stat_get_db_blk_write_time */
-PG_STAT_GET_DBENTRY_FLOAT8(blk_write_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(blk_write_time)
=20
 /* pg_stat_get_db_idle_in_transaction_time */
-PG_STAT_GET_DBENTRY_FLOAT8(idle_in_transaction_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(idle_in_transaction_time)
=20
 /* pg_stat_get_db_session_time */
-PG_STAT_GET_DBENTRY_FLOAT8(session_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(session_time)
=20
 Datum
 pg_stat_get_bgwriter_timed_checkpoints(PG_FUNCTION_ARGS)

--wJjppPOwlPKC1yZ0--

--8SCvqy49/qnuJgUv
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmQg72IACgkQnvQgOdby
QH3hHQ//Un49uvdRLcerHIHy31hVIhvucoVG6gChBw+zx6bbUIZUitls5OPlieKo
lGWmoskPCgv9mHA+UkKi2w3C0QuhLJ7vgTJega4C3HHQNivV9uRBe/gJtuM12AAs
XI0ir20jcJoc3KT5EcOIXUT6U3y0R9bP+SD9L5FofwGaYN9nYyO7Vu7mnJ/xDXgP
cI2H3Lha1nXYWLxY1EPP5R+fWJEk8BDUd73ECw2DlLjRyqpgnF0F6URketACej/a
HNusgv+pPaImHFRF9JXcQQ6Fi50+tk/f1XZ9jCHv3gqNh1EDf24WGuBmPlxx2RBc
tWn8ljHD3wIzpEHz3A3m8mEyf1KUcPvDV7hI168fYk7my9nJleyos+yWvbVAYRo7
TYcbwp97MjUl3E9GlLJBb18LYI6BNVUd0fFdXXIJcqC9VS4iEWxW+jh10MFMV2DE
lgScmJp+HyLMDyXRiAUWH1Ndk1ymh4HFuPYzcia787zu4eVWYBTtPtitPZOVnjCL
J8AfRe+AnrPLR2FrEiYAAoTdC6E1smmrtHVt6RDkT4xxQSPwPhn9/bWUSg6oSkxz
4KrkUhLYouWw3+o1bnSbxIN4OIW7kYInu0VPrG3j3J2sOHbNWRkYtAytF/W7pnII
YHKCaUi7/0x6Ykyc0HVK2sN0bYG94JOwlu3E5ctGsI6D+3N+5zM=
=WQWD
-----END PGP SIGNATURE-----

--8SCvqy49/qnuJgUv--