[PATCH] Change wait_time column of pg_stat_lock to double precision

Started by Tatsuya Kawataabout 1 month ago14 messageshackers
Jump to latest
#1Tatsuya Kawata
kawatatatsuya0913@gmail.com

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
#2Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Tatsuya Kawata (#1)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#3Tatsuya Kawata
kawatatatsuya0913@gmail.com
In reply to: Kyotaro Horiguchi (#2)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#4Bertrand Drouvot
bertranddrouvot.pg@gmail.com
In reply to: Kyotaro Horiguchi (#2)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#5Tatsuya Kawata
kawatatatsuya0913@gmail.com
In reply to: Bertrand Drouvot (#4)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#6Bertrand Drouvot
bertranddrouvot.pg@gmail.com
In reply to: Tatsuya Kawata (#5)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#7Tatsuya Kawata
kawatatatsuya0913@gmail.com
In reply to: Bertrand Drouvot (#6)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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
#8Michael Paquier
michael@paquier.xyz
In reply to: Tatsuya Kawata (#7)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#9Bertrand Drouvot
bertranddrouvot.pg@gmail.com
In reply to: Michael Paquier (#8)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#10Michael Paquier
michael@paquier.xyz
In reply to: Bertrand Drouvot (#9)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

Maybe 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

#11Bertrand Drouvot
bertranddrouvot.pg@gmail.com
In reply to: Michael Paquier (#10)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

Maybe 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

#12Bertrand Drouvot
bertranddrouvot.pg@gmail.com
In reply to: Bertrand Drouvot (#11)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

Maybe 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

#13Michael Paquier
michael@paquier.xyz
In reply to: Bertrand Drouvot (#9)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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

#14Tatsuya Kawata
kawatatatsuya0913@gmail.com
In reply to: Michael Paquier (#13)
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

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