Date: Fri, 5 Jun 2026 13:09:37 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
	Robert Haas <robertmhaas@gmail.com>,
	Roman Eskin <r.eskin@arenadata.io>,
	Michael Paquier <michael@paquier.xyz>,
	Alexander Lakhin <exclusion@gmail.com>,
	pgsql-hackers@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: Avoid orphaned objects dependencies, take 3
Message-ID: <aiLKkTC6QBt8i35P@bdtpg>
References: <9f5f0dd4-96d5-4d90-9436-62d96e0298f9@arenadata.io>
 <ad/awXLnMuc6Rf5M@bdtpg>
 <afCXPVNuvpf5ECGZ@bdtpg>
 <CA+TgmobtzkG9HzGtDXbgb3KVE46qHuSvGcuKHgAGQ1aejCoLLw@mail.gmail.com>
 <agsCqlLTytZCudMv@bdtpg>
 <aebf583b-b904-4035-a1c1-a4c5ad064dec@iki.fi>
 <ahYAKgbx05IQdjwR@bdtpg>
 <3963e573-4852-4d25-b741-7c5c7e1f9140@iki.fi>
 <ah1PIyqtT+427dWb@bdtpg>
 <02e28438bd448ff4ee8f6fd78e6b74e657172d73.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="9vJ2bamBnv+Gh0A+"
Content-Disposition: inline
In-Reply-To: <02e28438bd448ff4ee8f6fd78e6b74e657172d73.camel@j-davis.com>
Archived-At: <https://www.postgresql.org/message-id/aiLKkTC6QBt8i35P%40bdtpg>
Precedence: bulk


--9vJ2bamBnv+Gh0A+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

On Thu, Jun 04, 2026 at 10:03:22AM -0700, Jeff Davis wrote:
> On Mon, 2026-06-01 at 09:21 +0000, Bertrand Drouvot wrote:
> > The tracking array lives in a dedicated AclCheckTrackContext memory
> > context
> > (child of TopMemoryContext). The context is reset at the start of
> > each
> > top-level utility statement, which frees all prior allocations and
> > provides
> > clean lifetime management.
> > 
> > Recording is gated by aclcheck_tracking_active, which is set to true
> > only
> > during top-level utility statement execution. This ensures DML and
> > queries pay
> > no cost. The flag is cleared both at normal completion of
> > ProcessUtility and in
> > AbortTransaction to handle the error path.
> 
> This could use some better high-level comments in the code. Something
> like:
> 
> "DDL performs ACL checks on referenced objects before acquiring a lock
> on them. The lock is acquired much later, when recording dependencies.
> Track the ACL checks, so that we can re-check them after acquiring the
> lock.

Agreed, I just re-worded a bit to add some nuance in v24 attached, as:

"
may perform ACL checks on referenced objects without first holding a lock on
them. In that case, the lock is acquired much later, when recording the
dependencies. Track the ACL checks, so that we can re-check them after acquiring
the lock while recording dependencies."

> XXX: consider refactoring so that we perform the name lookup,
> acquire the lock, and check ACLs all in unison, like
> RangeVarGetRelidExtended()."

I like the XXX, as I agree that what you mentioned in [1] is an area of improvement
but not something directly linked to the TOCTOU issue that this patch is addressing.

FWIW, with the scenario you described in [1]:

- before 2fbb21170e9, the function would be linked to a non existing schema
- with 2fbb21170e9, it produces "ERROR:  referenced schema was concurrently dropped"

the current patch just "keep" the post 2fbb21170e9 behavior.

[1]: https://postgr.es/m/5315d15a42109297259d1a3264ad09e363eb98df.camel%40j-davis.com

Regards,

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

--9vJ2bamBnv+Gh0A+
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v24-0001-Recheck-permissions-after-lock-acquisition-in-de.patch"