[PATCH] Change wait_time column of pg_stat_lock to double precision
Hi,
While looking at the lock-related code, I noticed that pg_stat_lock
is the only statistics view whose timing column (wait_time) uses
bigint. Every other statistics view uses double precision for
measured-time columns. I do not see a reason for pg_stat_lock to
differ.
The attached patch changes the column to double precision.
Regards,
Tatsuya Kawata
Attachments:
v1-0001-Change-wait_time-column-of-pg_stat_lock-to-double.patchapplication/octet-stream; name=v1-0001-Change-wait_time-column-of-pg_stat_lock-to-double.patchDownload+10-10
Hello.
At Mon, 15 Jun 2026 15:54:24 +0900, Tatsuya Kawata <kawatatatsuya0913@gmail.com> wrote in
While looking at the lock-related code, I noticed that pg_stat_lock
is the only statistics view whose timing column (wait_time) uses
bigint. Every other statistics view uses double precision for
measured-time columns. I do not see a reason for pg_stat_lock to
differ.
It seems to me that this was intentional. As described in the
documentation, since wait_time is only accumulated for waits longer
than deadlock_timeout, sub-millisecond precision was probably not
considered particularly useful.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Hi Horiguchi-san,
Thanks for the review!
since wait_time is only accumulated for waits longer
than deadlock_timeout, sub-millisecond precision was probably not
considered particularly useful.
You are right that, given the deadlock_timeout floor (which is a
GUC, but defaults to one second), each individual wait is already
on the order of one second, so the sub-millisecond remainder is at
most about 0.1% of the value being recorded. The precision
argument is therefore weak on its own, and I do not want to insist
on it.
The primary motivation I had in mind is consistency across the
statistics views: pg_stat_lock is the only one whose timing column
is not double precision. Even if the bigint choice was deliberate,
the view ends up breaking the pattern.
If we decide to keep bigint, I think it would be worth documenting
the reason explicitly (e.g. that wait_time is only accumulated
for waits longer than deadlock_timeout, hence sub-millisecond
precision is not needed).
Thoughts?
Regards,
Tatsuya Kawata
Hi,
On Mon, Jun 15, 2026 at 04:39:22PM +0900, Kyotaro Horiguchi wrote:
Hello.
At Mon, 15 Jun 2026 15:54:24 +0900, Tatsuya Kawata <kawatatatsuya0913@gmail.com> wrote in
While looking at the lock-related code, I noticed that pg_stat_lock
is the only statistics view whose timing column (wait_time) uses
bigint. Every other statistics view uses double precision for
measured-time columns. I do not see a reason for pg_stat_lock to
differ.It seems to me that this was intentional. As described in the
documentation, since wait_time is only accumulated for waits longer
than deadlock_timeout, sub-millisecond precision was probably not
considered particularly useful.
Yeah that was intentional (and for the reason you described above).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi Bertrand-san, Horiguchi-san,
Thanks for confirming the original intent.
Before we conclude, I would like to share a couple of points that
make me wonder whether changing the type might still be worth
considering.
1. pg_stat_lock is new in v19, so the type can still be changed
before release without any backwards-compatibility cost. This
makes now a relatively low-risk moment to revisit the choice.
2. Looking across the other stats views, the "sub-millisecond
precision is not particularly useful" criterion does not seem to
be the basis for picking a type in general.
pg_stat_database.session_time, for example, can accumulate to
large values for which sub-millisecond precision is also noise,
yet it uses double precision. From a user's point of view,
the common pattern across the stats views seems to be
"measured time columns are double precision", regardless of
expected magnitude or required precision.
3. As a minor point, deadlock_timeout is a GUC and can be lowered,
so under diagnostic configurations sub-millisecond precision
in wait_time is not entirely hypothetical.
So my point is not that the original bigint choice was wrong, but
that pg_stat_lock currently differs from the other stats views in
this respect, and v19 may be a good moment to make it uniform.
If the consensus after considering these points is still that the
existing bigint type is preferable, I am happy to withdraw and send
a docs-only patch making the rationale explicit instead.
Regards,
Tatsuya Kawata
Hi Kawata-san,
On Sat, Jun 20, 2026 at 03:44:52PM +0900, Tatsuya Kawata wrote:
Hi Bertrand-san, Horiguchi-san,
Thanks for confirming the original intent.
Before we conclude, I would like to share a couple of points that
make me wonder whether changing the type might still be worth
considering.1. pg_stat_lock is new in v19, so the type can still be changed
before release without any backwards-compatibility cost. This
makes now a relatively low-risk moment to revisit the choice.2. Looking across the other stats views, the "sub-millisecond
precision is not particularly useful" criterion does not seem to
be the basis for picking a type in general.
pg_stat_database.session_time, for example, can accumulate to
large values for which sub-millisecond precision is also noise,
yet it uses double precision. From a user's point of view,
the common pattern across the stats views seems to be
"measured time columns are double precision", regardless of
expected magnitude or required precision.3. As a minor point, deadlock_timeout is a GUC and can be lowered,
so under diagnostic configurations sub-millisecond precision
in wait_time is not entirely hypothetical.So my point is not that the original bigint choice was wrong, but
that pg_stat_lock currently differs from the other stats views in
this respect, and v19 may be a good moment to make it uniform.
I can see your points though I'm not fully convinced yet. OTOH, would that
hurt to be consistent (even if, I think, it does not really provide real
actionable added value).
If the consensus after considering these points is still that the
existing bigint type is preferable, I am happy to withdraw and send
a docs-only patch making the rationale explicit instead.
If we were to update something then I think I'd prefer to change the code.
So, looking at your v1:
=== 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?
and so:
=== 2
- pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs);
+ pgstat_count_lock_waits(locallock->tag.lock.locktag_type,
+ (double) msecs + (double) usecs / 1000.0);
would:
- msecs = secs * 1000 + usecs / 1000;
- usecs = usecs % 1000;
/* Increment the lock statistics counters if done waiting. */
if (myWaitStatus == PROC_WAIT_STATUS_OK)
- pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs);
+ pgstat_count_lock_waits(locallock->tag.lock.locktag_type, secs * 1000000 + usecs);
+
+ msecs = secs * 1000 + usecs / 1000;
+ usecs = usecs % 1000;
=== 3
- values[i++] = Int64GetDatum(lck_stats->wait_time);
+ values[i++] = Float8GetDatum(lck_stats->wait_time);
Then, what about doing:
values[i++] = Float8GetDatum((double) lck_stats->wait_time / 1000.0);
instead?
=== 4
and instead of:
- PgStat_Counter wait_time; /* time in milliseconds */
+ double wait_time; /* time in milliseconds */
only change the comment here: to microseconds (but keep PgStat_Counter as type).
The idea being to keep the PgStat_Counter type, the long parameter type and
do the conversion at display time.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi Bertrand-san,
Thank you for your review!
=== 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.
=== 2
- pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs); + pgstat_count_lock_waits(locallock->tag.lock.locktag_type, + (double) msecs + (double) usecs /
1000.0);
would:
- msecs = secs * 1000 + usecs / 1000;
- usecs = usecs % 1000;/* Increment the lock statistics counters if done
waiting. */
if (myWaitStatus == PROC_WAIT_STATUS_OK)
-
pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs);
+
pgstat_count_lock_waits(locallock->tag.lock.locktag_type, secs * 1000000 +
usecs);
+ + msecs = secs * 1000 + usecs / 1000; + usecs = usecs % 1000;
Agreed. Fixed.
=== 3
- values[i++] = Int64GetDatum(lck_stats->wait_time); + values[i++] = Float8GetDatum(lck_stats->wait_time);Then, what about doing:
values[i++] = Float8GetDatum((double) lck_stats->wait_time / 1000.0);
instead?
Agreed. Fixed.
=== 4
and instead of:
- PgStat_Counter wait_time; /* time in milliseconds */ + double wait_time; /* time in milliseconds */only change the comment here: to microseconds (but keep PgStat_Counter as
type).
The idea being to keep the PgStat_Counter type, the long parameter type
and
do the conversion at display time.
Agreed. Fixed.
v2 attached.
Regards,
Tatsuya Kawata
Attachments:
v2-0001-Change-wait_time-column-of-pg_stat_lock-to-double.patchapplication/octet-stream; name=v2-0001-Change-wait_time-column-of-pg_stat_lock-to-double.patchDownload+13-13
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
Hi,
On Mon, Jun 29, 2026 at 11:11:06AM +0900, Michael Paquier wrote:
On Sun, Jun 28, 2026 at 02:50:38AM +0900, Tatsuya Kawata wrote:
Thank you for your review!
-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).
That makes sense and the lock time that would produce the overflow is realistic
(about 35 minutes) if my math is correct.
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.
Right. That said I just observed that it can produces things like:
postgres=# select read_time from pg_stat_io where read_time > 0;
read_time
---------------------
2.2640000000000002
0.08700000000000001
Maybe it should be changed to?
return (double) val_ms / 1000.0;
instead of:
return val_ms * (double) 0.001;
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?
Yeah, so this change.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote:
postgres=# select read_time from pg_stat_io where read_time > 0;
read_time
---------------------
2.2640000000000002
0.08700000000000001Maybe it should be changed to?
pg_stat_io with track_io_timing enabled shows the same thing. FWIW,
that does not bother me much. It's just more important to me to apply
the same formulas across the board.
--
Michael
Hi,
On Mon, Jun 29, 2026 at 01:52:54PM +0900, Michael Paquier wrote:
On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote:
postgres=# select read_time from pg_stat_io where read_time > 0;
read_time
---------------------
2.2640000000000002
0.08700000000000001Maybe it should be changed to?
pg_stat_io with track_io_timing enabled shows the same thing.
Yeah, I agree, my query above is querying pg_stat_io. I agree that it makes
sense to have both using pg_stat_us_to_ms(), I'm just wondering if pg_stat_us_to_ms()
should be changed as I did propose.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Mon, Jun 29, 2026 at 04:56:11AM +0000, Bertrand Drouvot wrote:
Hi,
On Mon, Jun 29, 2026 at 01:52:54PM +0900, Michael Paquier wrote:
On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote:
postgres=# select read_time from pg_stat_io where read_time > 0;
read_time
---------------------
2.2640000000000002
0.08700000000000001Maybe it should be changed to?
pg_stat_io with track_io_timing enabled shows the same thing.
Yeah, I agree, my query above is querying pg_stat_io. I agree that it makes
sense to have both using pg_stat_us_to_ms(), I'm just wondering if pg_stat_us_to_ms()
should be changed as I did propose.
FWIW, I just created a dedicated thread [1]/messages/by-id/akIYkMK4bHe9qX/N@bdtpg for this (to not deviate from the
original purpose of this one).
[1]: /messages/by-id/akIYkMK4bHe9qX/N@bdtpg
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote:
Hi,
On Mon, Jun 29, 2026 at 11:11:06AM +0900, Michael Paquier wrote:
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).That makes sense and the lock time that would produce the overflow is realistic
(about 35 minutes) if my math is correct.
Dealing with the "long" change, I have arrived at the conclusion that
using int64 or even uint64 felt stupid because we already use a cast
to PgStat_Counter in pgstat_count_lock_waits(), and some other stats
just use PgStat_Counter when reporting usec times. So I have just
switched the code to use PgStat_Counter.
The counter is stored in usecs, displayed in msecs. Documenting it as
stored in msecs is incorrect, no?Yeah, so this change.
I need to re-learn how to read.
And backpatched down to v19, catversion bump oblige.
--
Michael
Hi Michael-san,
Sorry for the late reply.
And backpatched down to v19, catversion bump oblige.
So I have just switched the code to use PgStat_Counter.
Thanks for picking this up and committing it, and also for catching the
long overflow issue on Win32 along the way.
Thanks also to Bertrand-san and Horiguchi-san for the review and discussion.
Regards,
Tatsuya Kawata