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: > > The motivation makes sense to me.
Thanks for looking at it and sharing your thoughts! > > 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. 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. > > 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. > Other than the few comments I had, this patchset looks good. It follows > patterns that were already established with the per-backend IO and WAL > stats. Thanks! Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
