Hi Scott, I reviewed the v6 and have some comments.
pgstat_get_xact_start_by_proc_number() reads BackendStatusArray directly, so the value is no longer taken from a cached snapshot that could predate the procarray pass by an arbitrary amount. The residual race you document in the header comment is a much smaller window, and the change-count retry loop and the pid <= 0 guard both look right. I also checked that p->procNumber = pgprocno is the right ProcNumber for indexing the status array, and that a prepared xact's dummy proc can't reach the array out of bounds, the pid <= 0 early return catches it before the bounds check does. The maintenance.sgml rewrite is better than what I suggested. Sorting all rows by age(shared_xmin) DESC NULLS LAST, and explaining that datfrozenxid can never advance past the shared horizon and that every ow's shared_xmin feeds it including backends in other databases, avoids the datid scoping question entirely rather than trying to describe it. The datfrozenxid clarification is a good catch too. One question. pg_stat_get_activity() gates st_xact_start_timestamp behind HAS_PGSTAT_PERMISSIONS, but the new helper applies no such check. That isn't reachable today, since both the view and the SRF require pg_read_all_stats, and such a caller already passes that test for every backend. But it does mean the SRF omits a check pg_stat_activity applies. Bharath pointed out one asymmetry already i.e pg_stat_activity shows pid, datid, xid and xmin to unprivileged non-owners while pg_xmin_horizon shows nothing, and this would be one in the other direction if the ACL were ever loosened or EXECUTE granted directly. Would it be worth applying HAS_PGSTAT_PERMISSIONS per row in the SRF? Regards, Surya Poondla
