Remove one TimestampTzGetDatum call in pg_stat_get_io()
Hi hackers,
While working on the per backend I/O statistics patch ([1]/messages/by-id/ZtXR+CtkEVVE/LHF@ip-10-97-1-34.eu-west-3.compute.internal), I noticed that
there is an unnecessary call to TimestampTzGetDatum() in pg_stat_get_io() (
as the reset_time is already a Datum).
Please find attached a tiny patch to remove this unnecessary call.
[1]: /messages/by-id/ZtXR+CtkEVVE/LHF@ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Remove-one-TimestampTzGetDatum-call-in-pg_stat_ge.patchtext/x-diff; charset=us-asciiDownload
From 09f7815a1dfe066ee3652b4f962b3f575aeee88e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Fri, 6 Sep 2024 08:47:37 +0000
Subject: [PATCH v1] Remove one TimestampTzGetDatum call in pg_stat_get_io()
When setting the tuple's value in pg_stat_get_io() there is no need to call
TimestampTzGetDatum() as reset_time is already a Datum.
---
src/backend/utils/adt/pgstatfuncs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
100.0% src/backend/utils/adt/
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 97dc09ac0d..33c7b25560 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1408,7 +1408,7 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
values[IO_COL_BACKEND_TYPE] = bktype_desc;
values[IO_COL_CONTEXT] = CStringGetTextDatum(context_name);
values[IO_COL_OBJECT] = CStringGetTextDatum(obj_name);
- values[IO_COL_RESET_TIME] = TimestampTzGetDatum(reset_time);
+ values[IO_COL_RESET_TIME] = reset_time;
/*
* Hard-code this to the value of BLCKSZ for now. Future
--
2.34.1
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
While working on the per backend I/O statistics patch ([1]), I noticed that
there is an unnecessary call to TimestampTzGetDatum() in pg_stat_get_io() (
as the reset_time is already a Datum).
Hmm, TimestampTzGetDatum is not a no-op on 32-bit machines. If you're
correct about this, why are our 32-bit BF animals not crashing? Are
we failing to test this code?
regards, tom lane
I wrote:
Hmm, TimestampTzGetDatum is not a no-op on 32-bit machines. If you're
correct about this, why are our 32-bit BF animals not crashing? Are
we failing to test this code?
Oh, I had the polarity backwards: this error doesn't result in trying
to dereference something that's not a pointer, but rather in
constructing an extra indirection layer, with the end result being
that the timestamp displayed in the pg_stat_io view is garbage
(I saw output like "1999-12-31 19:11:45.880208-05" while testing in
a 32-bit VM). It's not so surprising that our regression tests are
insensitive to the values being displayed there.
I confirm that this fixes the problem. Will push shortly.
regards, tom lane
Hi,
On Fri, Sep 06, 2024 at 11:40:56AM -0400, Tom Lane wrote:
I wrote:
Hmm, TimestampTzGetDatum is not a no-op on 32-bit machines. If you're
correct about this, why are our 32-bit BF animals not crashing? Are
we failing to test this code?Oh, I had the polarity backwards: this error doesn't result in trying
to dereference something that's not a pointer, but rather in
constructing an extra indirection layer, with the end result being
that the timestamp displayed in the pg_stat_io view is garbage
(I saw output like "1999-12-31 19:11:45.880208-05" while testing in
a 32-bit VM). It's not so surprising that our regression tests are
insensitive to the values being displayed there.I confirm that this fixes the problem. Will push shortly.
Thanks! Yeah was going to reply that that would display incorrect results on
32-bit (and not crashing). And since the tests don't check the values then
we did not notice.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com