Horton

Thread

Commits

  1. Fix incorrect pg_stat_io output on 32-bit machines.

  1. Remove one TimestampTzGetDatum call in pg_stat_get_io()

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2024-09-06T10:48:25Z

    Hi hackers,

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).

Please find attached a tiny patch to remove this unnecessary call.


[1]: https://www.postgresql.org/message-id/flat/ZtXR%2BCtkEVVE/LHF%40ip-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

  2. Re: Remove one TimestampTzGetDatum call in pg_stat_get_io()

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-09-06T14:38:18Z

    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

  3. Re: Remove one TimestampTzGetDatum call in pg_stat_get_io()

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-09-06T15:40:56Z

    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

  4. Re: Remove one TimestampTzGetDatum call in pg_stat_get_io()

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2024-09-06T15:45:27Z

    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