On Sun, Jun 28, 2026 at 02:50:38AM +0900, Tatsuya Kawata wrote:
> Thank you for your review!

Catching up with the business happening here..

I'm actually convinced by this patch, based on the point that
pg_stat_lock is the only stats-related view that does not use double
precision for the report.  All the others use float8.  Let's adjust it
now as it is not released yet.

>> === 1
>>
>> -pgstat_count_lock_waits(uint8 locktag_type, long msecs)
>> +pgstat_count_lock_waits(uint8 locktag_type, double msecs)
>>
>> What about keeping the long and rename to usecs?
> 
> Agreed. Fixed.

Relying on long here is not reliable on WIN32, where the value could
overflow for long wait times (where sizeof(long) == 4).  I'd suggest a
int64 or a uint64 instead for the API to be able to store wait times
in usecs longer than the overflow threshold.  That was another design
oversight.  My oversight here (aka I want to abolish completely the
use of long in the core code; it leads to insanity).

>> values[i++] = Float8GetDatum((double) lck_stats->wait_time / 1000.0);
>>
>> instead?
> 
> Agreed. Fixed.

We have a pg_stat_us_to_ms() that can do this job as well for the
values displayed.  Why duplicate this code?  This conversion routine
is used by pg_stat_io.

>> === 4
>>
>> The idea being to keep the PgStat_Counter type, the long parameter type
> and
>> do the conversion at display time.
> 
> Agreed. Fixed.

-   PgStat_Counter wait_time;   /* time in milliseconds */
+   PgStat_Counter wait_time;   /* time in microseconds */

The counter is stored in usecs, displayed in msecs.  Documenting it as
stored in msecs is incorrect, no?
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to