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+1-2
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