Date: Thu, 24 Sep 2026 05:27:26 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: shihao zhong <zhong950419@gmail.com>,
	Jim Jones <jim.jones@uni-muenster.de>,
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add a permission check to pg_stat_get_backend_subxact()
Message-ID: <arS0vg/sLNmuT8T7@bdtpg>
References: 
 <CAGRkXqRGVNvvkY7a7X=3Bn=WgfLuFZ5iVsBf+t6HSfm2RwVC+g@mail.gmail.com>
 <aqec3MF_GDsMdadl@paquier.xyz>
 <arD/w47Ug1GaObfq@bdtpg>
 <arJJVOo1mOKvK8US@paquier.xyz>
 <arJSXjs6szWXIr0H@bdtpg>
 <arJlSpeSs8yPKipu@paquier.xyz>
 <arKKltvf8PpIByOF@bdtpg>
 <CAGRkXqTLuE3PrRn7tXJ8t+7UHcTUJ9cuaFxbZkuhpAySD0RpfA@mail.gmail.com>
 <arN+VT3xyiTY/iUD@bdtpg>
 <arR2TkF9msIowJjh@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <arR2TkF9msIowJjh@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/arS0vg/sLNmuT8T7%40bdtpg>
Precedence: bulk

Hi,

On Thu, Sep 24, 2026 at 10:01:02AM +0900, Michael Paquier wrote:
> On Wed, Sep 23, 2026 at 07:23:01AM +0000, Bertrand Drouvot wrote:
> >  typedef struct PgStat_Backend
> >  {
> > +       int                     pid;                    /* PID of the backend owning these stats */
> >         TimestampTz stat_reset_timestamp;
> > 
> > 0002 explicitly says that it is not intended for backpatching, but what about
> > 0001? If it is backpatched to v18, adding pid here changes the offsets of all
> > the existing fields.
> 
> The main use case of
> backend stats is for benchmarking and get numbers with longer-running
> connections, so as a whole I think that we are making a big issue of
> something that is not really one in practice.

I'm not sure I agree with that assumption. Per-backend statistics can also be
used for monitoring and diagnostics, including with high connection turnover.

That said, I agree that ProcNumber reuse while statistics are held should be rare
in practice.

> Note that there is a parallel with replication slot stats, which are
> indexed not by name but with an integer number.  A backend could grab
> in a snapshot data from slot 1, while concurrent activity has the idea
> to drop and recreate a slot.  The snapshot would still refer to the
> data of the previous slot.  If we aim at improving this kind of use
> cases with stats snapshots, and I am not sure that it's really worth
> bothering, this should work across all the stats kinds, not be plugged
> multiple times across the board.

Your replication slot point makes sense. If we want to address those, a generic
approach would probably be better than special handling for backend statistics.

Storing the user ID with the statistics is enough for the permission check, so
dropping the PID from v10-0001 makes sense to me.

Maybe worth adding a comment to pgstat_fetch_stat_backend_by_pid() mentioning that
cached statistics may belong to an older backend that used the same ProcNumber,
to avoid this being rediscovered later? (If so, I'll draft such a patch).

> The role ID case is different: we want consistency to check for the
> permissions.

Agreed. What about v10-0002? I think this is different. pgstat_read_current_status()
is constructing one activity snapshot entry from the activity entry and
PGPROC. Without the cross check, it may combine the PID and user ID of one
backend with the subxact counters of another one. Is the PID check there worth
keeping for HEAD?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com