Hi Scott, +1 on the feature. This is a nice thread with some good review comments. I applied v4, it builds clean with no new warnings.
A few comments: 1. maintenance.sgml and system-views.sgml contradict each other on datid, and I feel maintenance.sgml is wrong. It says to consider "only rows whose datid is the affected database or null", but ReplicationSlotsComputeRequiredXmin() aggregates catalog_xmin over every in-use, non-invalidated slot with no filter on s->data.database, and that folds into the shared and catalog horizons unconditionally. A logical slot on database A is reported with datid = A, so it can hold back database B's catalog horizon. system-views.sgml says the opposite and is right: "Slot and standby_feedback rows pin the horizon cluster-wide regardless of datid, and a query filtering by datid will exclude them.", maybe we can fix this. 2. Would a column distinguishing the xid holder from xmin holders be worth adding? Since the SRF folds p->xid and p->xmin into one value, a backend that owns the oldest xid and a backend that merely snapshotted it come out identical. For example, one idle-in-transaction backend owning the oldest xid, with twenty other backends whose backend_xmin equals it, produces twenty-one indistinguishable rows and system-views.sgml then says "When several rows tie for the oldest value, removing one of them does not advance the horizon", which I think inverts the advice for that case, since terminating the single xid owner is what actually helps. To be clear, I agree with your argument on the other thread that the view should list every input rather than nominate one. My only concern is that the role isn't recorded anywhere, so the fact a DBA most needs is the one the view can't convey. 3. It would be worth adding a test for the case where a slot's effective_xmin is set while data.xmin is still invalid, since that's the state pg_replication_slots cannot show. TAP case (3) asserts a logical slot with catalog_xmin non-null and data_xmin NULL, but pg_replication_slots reports the same for that slot. To reach the divergent state we need CREATE_REPLICATION_SLOT ... LOGICAL <plugin> (SNAPSHOT 'export') over a replication connection; pg_create_logical_replication_slot() passes need_full_snapshot = false, so it never pegs effective_xmin. Conveniently the state is stable and ReplicationSlotRelease() only clears effective_xmin when the slot is released, so another session can compare the two views while the replication connection is held open. A few minor things: - The recovery errhint() puts the function name inside the format string. We can use a placeholder to keep the name untranslated, like how pg_log_standby_snapshot() does. - The xact_start assertion in the regression test compares the view's xact_start against pg_stat_activity.xact_start, which is where the view sources it, so it exercises the join rather than the value. Regards, Surya Poondla
