On Fri Jun 5, 2026 at 8:29 AM UTC, Bertrand Drouvot wrote: > Hi, > > On Thu, Jun 04, 2026 at 09:32:39PM +0000, Tristan Partin wrote: >> On Wed Jun 3, 2026 at 1:59 PM UTC, Bertrand Drouvot wrote: >> > 0001: Refactor pg_stat_get_lock() to use a helper function >> >> > +static void >> > +pg_stat_lock_build_tuples(ReturnSetInfo *rsinfo, >> > + PgStat_LockEntry *lock_stats, >> > + TimestampTz stat_reset_timestamp) >> >> I think that the alignment of the second and third arguments could be >> off by one. They should line up with the capital R in ReturnSetInfo.
Probably just my editor being weird if it looks good to you! > They look ok to me in the C file, what about you? > >> > - values[i] = TimestampTzGetDatum(lock_stats->stat_reset_timestamp); >> > + if (stat_reset_timestamp != 0) >> > + values[i] = TimestampTzGetDatum(stat_reset_timestamp); >> > + else >> > + nulls[i] = true; >> >> It's not super clear to me why this changed in the first patch. > > It's to make less "noise" in the second patch and keep the second patch > focusing > only on the "new feature". It's to ease to review but could be merged before > being pushed would the commiter decides to do so. Sounds good. To me it made the review a little more difficult, but I understand the motivation. >> > 0002: Add per-backend lock statistics >> >> > + Returns lock statistics about the backend with the specified >> > + process ID. The output fields are exactly the same as the ones in >> > the >> > + <structname>pg_stat_lock</structname> view. >> >> It probably makes sense to link to pg_stat_lock here. > > Not sure as that would not be consistent with pg_stat_get_backend_io and > pg_stat_get_backend_wal descriptions in monitoring.sgml. Makes sense. Maybe we can update that in a future documentation update. I'll go ahead and submit something in a separate thread. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
