Thread

Commits

  1. Change stat_lock.wait_time to double precision

  1. [PATCH] Change wait_time column of pg_stat_lock to double precision

    Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-15T06:54:24Z

    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
    
  2. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-15T07:39:22Z

    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
    
    
    
    
  3. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-15T13:12:08Z

    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
    
  4. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-15T13:54:54Z

    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
    
    
    
    
  5. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-20T06:44:52Z

    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
    
  6. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-22T04:26:42Z

    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
    
    
    
    
  7. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-27T17:50:38Z

    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
    
  8. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Michael Paquier <michael@paquier.xyz> — 2026-06-29T02:11:06Z

    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
    
  9. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-29T04:27:55Z

    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
    
    
    
    
  10. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Michael Paquier <michael@paquier.xyz> — 2026-06-29T04:52:54Z

    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
    
  11. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-29T04:56:11Z

    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
    
    
    
    
  12. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-29T07:06:36Z

    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] for this (to not deviate from the
    original purpose of this one).
    
    [1]: https://postgr.es/m/akIYkMK4bHe9qX/N%40bdtpg
    
    Regards,
    
    -- 
    Bertrand Drouvot
    PostgreSQL Contributors Team
    RDS Open Source Databases
    Amazon Web Services: https://aws.amazon.com
    
    
    
    
  13. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Michael Paquier <michael@paquier.xyz> — 2026-06-30T03:48:50Z

    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
    
  14. Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

    Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-07-01T15:28:16Z

    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