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
