Date: Wed, 23 Sep 2026 07:23:01 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: shihao zhong <zhong950419@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	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: <arN+VT3xyiTY/iUD@bdtpg>
References: <c7a0be5d-5328-41dd-a7bf-6b964e9f7a06@uni-muenster.de>
 <aqSXMjhwFFzz_zyo@paquier.xyz>
 <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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAGRkXqTLuE3PrRn7tXJ8t+7UHcTUJ9cuaFxbZkuhpAySD0RpfA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/arN%2BVT3xyiTY/iUD%40bdtpg>
Precedence: bulk

Hi,

On Tue, Sep 22, 2026 at 11:22:40PM -0400, shihao zhong wrote:
> > Do you mean adding the user ID on top of Shihao's 0002? If so, I can have
> a look,
> > unless Shihao is already planning to update the patch along those lines?
> 
> Done in v8, attached.

Thanks!

I've a few comments:

> 0001 is the PID check, same as v7-0002. It goes first now because 0002
> needs a field in PgStat_Backend.
> 
> 0002 stores the user ID in PgStat_Backend next to the PID when the entry
> is created, and pg_stat_get_backend_io(), wal() and lock() check the
> caller against that instead of the beentry. The reset callback restores
> both fields. pg_stat_get_backend_subxact() still checks the beentry, the
> counters it reports come from there.

=== 1

pgstat_read_current_status() first copies the activity entry and then calls
ProcNumberGetTransactionIds() separately. If the backend exits and its
ProcNumber is reused in between, the userid can belong to the old backend
while the subxact counters belong to the new one.

This race exists before the patch, but it matters for the new permission check.
I wonder if we should pass the copied PID to ProcNumberGetTransactionIds() and
validate it under ProcArrayLock, following the same idea as
pgstat_fetch_stat_backend_by_pid()?

=== 2

 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.

I think it would make sense to add pid at the end in the backbranches (if we
back patch it), as suggested in [1]. That would preserve the existing field
offsets, though it would still change sizeof(PgStat_Backend). FWIW, I could not
find any use of sizeof(PgStat_Backend) in a GitHub code search and there is
no padding to add the new field into.

[1]: https://wiki.postgresql.org/wiki/Committing_checklist

Regards,

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