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