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