Subject: Re: Avoid orphaned objects dependencies, take 3

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 18 May 2026 12:14:34 +0000

Hi,

On Wed, May 13, 2026 at 04:20:21PM -0400, Robert Haas wrote:
> On Tue, Apr 28, 2026 at 7:17 AM Bertrand Drouvot
> <[email-masked]> wrote:
> > 0003: Add Assert guard to detect permission check before lock regressions
> >
> > Add instrumentation under USE_ASSERT_CHECKING to detect cases where object_aclcheck()
> > is called on a referenced object before a lock is held on it, which would widen
> > the TOCTOU window between the permission check and the dependency recording.
> 
> I really like the idea of having some kind of cross-check system that
> can detect future (or current) coding mistakes.

Thanks for the feedback! BTW, it detected a new one due to 4793fc41f82, so v21
attached does fix it to make the CI green.

> But what I wonder
> about this mechanism is: should we instead be insisting that we take a
> lock and check permissions on every dependency? Is it an error to
> record a dependency on an object without any sort of permissions
> check?

I'm not sure. For example, currently, without execute privilege on myfunc(), one
could create a view like:

CREATE VIEW v1 AS SELECT myfunc(a) FROM t1;

so that the dependency is recorded.

The execution permission is checked when the view is queried. I don't think this
example is a bug, so that I'm doubtful about insisting that we take a lock and
check permissions on every dependency.

> Also, I think the mechanism might not be entirely safe. ProcessUtility
> can result in executing user-defined functions which could
> theoretically run other DDL and then it seems like this code would get
> confused.

The assert fires when an aclcheck was tracked and the lock is not held. Since
the locks are transaction-scoped, any lock acquired during the statement persists,
so the assert passes regardless of nesting. So that looks not possible to get false
positives (assert fires) due to user-defined functions running other DDL.

The concern would be false negatives (missed detection) due to the reset wiping
the outer DDL's entries.

I believe, that's theoretically possible only if:

- The outer DDL has a P1 bug (aclcheck without lock on object X)
- A user-defined function happens to run DDL that also aclchecks and locks the same
object X
- The user-defined function DDL's lock satisfies the assert for the outer DDL

But then, the bug would be caught in normal testing without the user-defined
function being present.

So I'm not sure how this code could get confused. Do you have an example of what
you have in mind?

Regards,

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

---

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 26 May 2026 21:00:11 +0300

> attached does fix it to make the CI green.

I had a closer look at patch 0001. It doesn't fix all the problems, we 
definitely need patch 0002/0003 too, but it's a good step forward and I 
think it makes sense to commit it independently. So I'm focusing on that 
now.

I noticed we are already doing essentially the same thing for shared 
dependencies, in shdepLockAndCheckObject(). I see that you even copied 
the function comment from shdepLockAndCheckObject() to 
LockNotPinnedObject(), but I didn't see it being otherwise mentioned in 
this thread. In any case, that's a good argument for doing the same for 
non-shared dependencies that we already do for shared dependencies.

> +	if (object->classId == RelationRelationId)
> +	{
> +		/* skip shared relations as they are pinned */
> +		if (IsSharedRelation(object->objectId))
> +			return;
> +
> +		/*
> +		 * We must be in one of the two following cases that would already
> +		 * prevent the relation to be dropped: 1. The relation is already
> +		 * locked (could be an existing relation or a relation that we are
> +		 * creating). 2. The relation is protected indirectly (i.e an index
> +		 * protected by a lock on its table, a table protected by a lock on a
> +		 * function that depends of the table...). To avoid any risks, acquire
> +		 * a lock if there is none. That may add unnecessary lock for 2. but
> +		 * that's worth it.
> +		 */
> +		if (!CheckRelationOidLockedByMe(object->objectId, AccessShareLock, true))
> +			LockRelationOid(object->objectId, AccessShareLock);
> +		return;
> +	}

Hmm, shouldn't that re-check that the relation exists, after acquiring 
the lock, like the non-relation codepath does? Otherwise it's a little 
pointless to acquire the lock.

I did a bunch of little refactorings and ended up with the attached. 
Notable changes:

- I renamed and moved LockNotPinnedObject() into 
dependencyLockAndCheckObject(), for consistency with 
shdepLockAndCheckObject().

- Removed ObjectByIdExist(), inlined it into the caller. The argument to 
use SnapshotSelf or not seemed a bit too special to be generally useful

- Changed the error message and code to rhyme with the existing "role 
<OID> was concurrently dropped" errors. I didn't add the OID to the 
error message however, because that makes the testing hard. It'd be nice 
to have a general masking mechanism for OIDs and XIDs in error messages 
in tests, but now is not the time for that.

- Added a test for a dependency on a role too, to cover the existing 
shdepLockAndCheckObject() function. It's currently disabled because it 
prints the OID, though.

- Heikki
From d51523d9003c902f3968e174f158c27680eab427 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Mon, 27 Apr 2026 14:01:46 +0000
Subject: [PATCH v22 1/1] Avoid orphaned objects dependencies

Concurrent DDL can create orphaned dependencies in pg_depend, objects
referencing other objects that no longer exist. For example:

Scenario 1:

session 1: begin; drop schema schem;
session 2: create a function in the schema schem
session 1: commit;

With the above, the function created in session 2 would be linked to a non
existing schema.

Scenario 2:

session 1: begin; create a function in the schema schem
session 2: drop schema schem;
session 1: commit;

With the above, the function created in session 1 would be linked to a non
existing schema.

Fix by acquiring AccessShareLock on referenced objects when recording
dependencies. This conflicts with AccessExclusiveLock taken by DROP,
preventing the race. After acquiring the lock, verify the object still
exists, if it was dropped concurrently, report an error.

The lock and check is done in both recordMultipleDependencies() and
changeDependencyFor().

The patch adds a few tests for some dependency cases (that would currently produce
orphaned objects):

- schema and function (as the above scenarios)
- alter a dependency (function and schema)
- function and arg type
- function and return type
- function and function
- domain and domain
- table and type
- server and foreign data wrapper

Author: Bertrand Drouvot <[email-masked]>
Reviewed-by: Heikki Linnakangas <[email-masked]>
Discussion: https://postgr.es/m/[email-masked]
---
 src/backend/catalog/pg_depend.c               | 130 +++++++++++++++++
 .../expected/test_dependencies_locks.out      | 137 ++++++++++++++++++
 src/test/isolation/isolation_schedule         |   1 +
 .../specs/test_dependencies_locks.spec        | 108 ++++++++++++++
 src/test/regress/expected/alter_table.out     |  11 +-
 5 files changed, 382 insertions(+), 5 deletions(-)
 create mode 100644 src/test/isolation/expected/test_dependencies_locks.out
 create mode 100644 src/test/isolation/specs/test_dependencies_locks.spec

diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 07c2d41c189..ff17d5850c5 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -19,6 +19,7 @@
 #include "access/table.h"
 #include "catalog/catalog.h"
 #include "catalog/dependency.h"
+#include "catalog/index.h"
 #include "catalog/indexing.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_depend.h"
@@ -27,13 +28,17 @@
 #include "catalog/partition.h"
 #include "commands/extension.h"
 #include "miscadmin.h"
+#include "storage/lmgr.h"
+#include "storage/lock.h"
 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
+#include "utils/snapmgr.h"
 #include "utils/syscache.h"

 static bool isObjectPinned(const ObjectAddress *object);
+static void dependencyLockAndCheckObject(Oid classId, Oid objectId);

 /*
@@ -109,6 +114,12 @@ recordMultipleDependencies(const ObjectAddress *depender,
 		if (isObjectPinned(referenced))
 			continue;
 
+		/*
+		 * Acquire a lock and check object still exists while recording the
+		 * dependency.
+		 */
+		dependencyLockAndCheckObject(referenced->classId, referenced->objectId);
+
 		if (slot_init_count < max_slots)
 		{
 			slot[slot_stored_count] = MakeSingleTupleTableSlot(RelationGetDescr(dependDesc),
@@ -507,6 +518,13 @@ changeDependencyFor(Oid classId, Oid objectId,
 		return 1;
 	}
 
+	/*
+	 * Make sure the new referenced object doesn't go away while we record the
+	 * dependency.
+	 */
+	if (!newIsPinned)
+		dependencyLockAndCheckObject(refClassId, newRefObjectId);
+
 	depRel = table_open(DependRelationId, RowExclusiveLock);
 
 	/* There should be existing dependency record(s), so search. */
@@ -714,6 +732,118 @@ isObjectPinned(const ObjectAddress *object)
 }

+/*
+ * dependencyLockAndCheckObject
+ *
+ * Lock the object that we are about to record a dependency on.  After it's
+ * locked, verify that it hasn't been dropped while we weren't looking.  If
+ * the object has been dropped, an error is thrown.
+ *
+ * If the caller already holds a lock that conflicts with DROP
+ * (AccessShareLock or stronger), this does nothing.  That is the desired case
+ * - callers should acquire the lock when they look up the object already -
+ * but many callers are skipping it currently.  This is a backstop to make
+ * that we don't record a bogus reference permanently in the catalogs.  In the
+ * future, after we have tightened up all the callers, this could just verify
+ * that all the objects are indeed locked and throw an error if not.
+ */
+static void
+dependencyLockAndCheckObject(Oid classId, Oid objectId)
+{
+	/*
+	 * Pinned objects cannot be dropped concurrently, and callers checked this
+	 * already.
+	 */
+	Assert(!IsPinnedObject(classId, objectId));
+
+	if (classId != RelationRelationId)
+	{
+		LOCKTAG		tag;
+		SysCacheIdentifier cache;
+		Relation rel;
+		SysScanDesc scan;
+		ScanKeyData skey;
+		HeapTuple tuple;
+
+		SET_LOCKTAG_OBJECT(tag,
+						   MyDatabaseId,
+						   classId,
+						   objectId,
+						   0);
+
+		if (LockHeldByMe(&tag, AccessShareLock, true))
+			return;
+
+		/* Assume we should lock the whole object not a sub-object */
+		LockDatabaseObject(classId, objectId, 0, AccessShareLock);
+
+		/*
+		 * Check that the object still exists.  If the catalog has a suitable
+		 * syscache, check that first.
+		 */
+		cache = get_object_catcache_oid(classId);
+		if (cache != SYSCACHEID_INVALID)
+		{
+			if (SearchSysCacheExists1(cache, ObjectIdGetDatum(objectId)))
+				return;
+		}
+
+		/*
+		 * If it's not found in the syscache, or there's no suitable syscache
+		 * we can use, scan the catalog table using SnapshotSelf.  This
+		 * handles the case that it's an object we just created (for example,
+		 * if it's a composite type created as part of creating a table).
+		 */
+		rel = table_open(classId, AccessShareLock);
+
+		ScanKeyInit(&skey,
+					get_object_attnum_oid(classId),
+					BTEqualStrategyNumber, F_OIDEQ,
+					ObjectIdGetDatum(objectId));
+
+		scan = systable_beginscan(rel, get_object_oid_index(classId),
+								  true, SnapshotSelf, 1, &skey);
+
+		tuple = systable_getnext(scan);
+		if (!HeapTupleIsValid(tuple))
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_OBJECT),
+					 errmsg("dependent %s was concurrently dropped",
+							get_object_class_descr(classId))));
+
+		systable_endscan(scan);
+		table_close(rel, AccessShareLock);
+	}
+	else
+	{
+		/*
+		 * Same logic for pg_class entries, but locking relations is handled
+		 * by different functions.
+		 *
+		 * Callers are more careful with locking relations than other objects,
+		 * so we should already have a lock on the relation, or on another
+		 * object that indirectly prevents the relation from being dropped.
+		 * For example, we might have a strong lock on a table while adding
+		 * dependency to its index.  However, we cannot detect the indirectly
+		 * protected case here easily.  To err on the safe side, acquire a
+		 * lock directly on the relation if we're not holding one already.
+		 */
+
+		/* all shared relations are pinned */
+		Assert(!IsSharedRelation(objectId));
+
+		if (CheckRelationOidLockedByMe(objectId, AccessShareLock, true))
+			return;
+		LockRelationOid(objectId, AccessShareLock);
+
+		if (SearchSysCacheExists1(RELOID, ObjectIdGetDatum(objectId)))
+			return;
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("dependent relation was concurrently dropped")));
+	}
+}
+
 /*
  * Various special-purpose lookups and manipulations of pg_depend.
  */
diff --git a/src/test/isolation/expected/test_dependencies_locks.out b/src/test/isolation/expected/test_dependencies_locks.out
new file mode 100644
index 00000000000..3bf9e435725
--- /dev/null
+++ b/src/test/isolation/expected/test_dependencies_locks.out
@@ -0,0 +1,137 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_begin s1_create_function_in_schema s2_drop_schema s1_commit
+step s1_begin: BEGIN;
+step s1_create_function_in_schema: CREATE FUNCTION testschema.foo() RETURNS int AS 'select 1' LANGUAGE sql;
+step s2_drop_schema: DROP SCHEMA testschema; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_schema: <... completed>
+ERROR:  cannot drop schema testschema because other objects depend on it
+
+starting permutation: s2_begin s2_drop_schema s1_create_function_in_schema s2_commit
+step s2_begin: BEGIN;
+step s2_drop_schema: DROP SCHEMA testschema;
+step s1_create_function_in_schema: CREATE FUNCTION testschema.foo() RETURNS int AS 'select 1' LANGUAGE sql; <waiting ...>
+step s2_commit: COMMIT;
+step s1_create_function_in_schema: <... completed>
+ERROR:  dependent schema was concurrently dropped
+
+starting permutation: s1_begin s1_alter_function_schema s2_drop_alterschema s1_commit
+step s1_begin: BEGIN;
+step s1_alter_function_schema: ALTER FUNCTION public.falter() SET SCHEMA alterschema;
+step s2_drop_alterschema: DROP SCHEMA alterschema; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_alterschema: <... completed>
+ERROR:  cannot drop schema alterschema because other objects depend on it
+
+starting permutation: s2_begin s2_drop_alterschema s1_alter_function_schema s2_commit
+step s2_begin: BEGIN;
+step s2_drop_alterschema: DROP SCHEMA alterschema;
+step s1_alter_function_schema: ALTER FUNCTION public.falter() SET SCHEMA alterschema; <waiting ...>
+step s2_commit: COMMIT;
+step s1_alter_function_schema: <... completed>
+ERROR:  dependent schema was concurrently dropped
+
+starting permutation: s1_begin s1_create_function_with_argtype s2_drop_foo_type s1_commit
+step s1_begin: BEGIN;
+step s1_create_function_with_argtype: CREATE FUNCTION fooargtype(num foo) RETURNS int AS 'select 1' LANGUAGE sql;
+step s2_drop_foo_type: DROP TYPE public.foo; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_foo_type: <... completed>
+ERROR:  cannot drop type foo because other objects depend on it
+
+starting permutation: s2_begin s2_drop_foo_type s1_create_function_with_argtype s2_commit
+step s2_begin: BEGIN;
+step s2_drop_foo_type: DROP TYPE public.foo;
+step s1_create_function_with_argtype: CREATE FUNCTION fooargtype(num foo) RETURNS int AS 'select 1' LANGUAGE sql; <waiting ...>
+step s2_commit: COMMIT;
+step s1_create_function_with_argtype: <... completed>
+ERROR:  dependent type was concurrently dropped
+
+starting permutation: s1_begin s1_create_function_with_rettype s2_drop_foo_rettype s1_commit
+step s1_begin: BEGIN;
+step s1_create_function_with_rettype: CREATE FUNCTION footrettype() RETURNS id LANGUAGE sql RETURN 1;
+step s2_drop_foo_rettype: DROP DOMAIN id; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_foo_rettype: <... completed>
+ERROR:  cannot drop type id because other objects depend on it
+
+starting permutation: s2_begin s2_drop_foo_rettype s1_create_function_with_rettype s2_commit
+step s2_begin: BEGIN;
+step s2_drop_foo_rettype: DROP DOMAIN id;
+step s1_create_function_with_rettype: CREATE FUNCTION footrettype() RETURNS id LANGUAGE sql RETURN 1; <waiting ...>
+step s2_commit: COMMIT;
+step s1_create_function_with_rettype: <... completed>
+ERROR:  dependent type was concurrently dropped
+
+starting permutation: s1_begin s1_create_function_with_function s2_drop_function_f s1_commit
+step s1_begin: BEGIN;
+step s1_create_function_with_function: CREATE FUNCTION foofunc() RETURNS int LANGUAGE SQL RETURN f() + 1;
+step s2_drop_function_f: DROP FUNCTION f(); <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_function_f: <... completed>
+ERROR:  cannot drop function f() because other objects depend on it
+
+starting permutation: s2_begin s2_drop_function_f s1_create_function_with_function s2_commit
+step s2_begin: BEGIN;
+step s2_drop_function_f: DROP FUNCTION f();
+step s1_create_function_with_function: CREATE FUNCTION foofunc() RETURNS int LANGUAGE SQL RETURN f() + 1; <waiting ...>
+step s2_commit: COMMIT;
+step s1_create_function_with_function: <... completed>
+ERROR:  dependent function was concurrently dropped
+
+starting permutation: s1_begin s1_create_domain_with_domain s2_drop_domain_id s1_commit
+step s1_begin: BEGIN;
+step s1_create_domain_with_domain: CREATE DOMAIN idid as id;
+step s2_drop_domain_id: DROP DOMAIN id; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_domain_id: <... completed>
+ERROR:  cannot drop type id because other objects depend on it
+
+starting permutation: s2_begin s2_drop_domain_id s1_create_domain_with_domain s2_commit
+step s2_begin: BEGIN;
+step s2_drop_domain_id: DROP DOMAIN id;
+step s1_create_domain_with_domain: CREATE DOMAIN idid as id; <waiting ...>
+step s2_commit: COMMIT;
+step s1_create_domain_with_domain: <... completed>
+ERROR:  dependent type was concurrently dropped
+
+starting permutation: s1_begin s1_create_table_with_type s2_drop_footab_type s1_commit
+step s1_begin: BEGIN;
+step s1_create_table_with_type: CREATE TABLE tabtype(a footab);
+step s2_drop_footab_type: DROP TYPE public.footab; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_footab_type: <... completed>
+ERROR:  cannot drop type footab because other objects depend on it
+
+starting permutation: s2_begin s2_drop_footab_type s1_create_table_with_type s2_commit
+step s2_begin: BEGIN;
+step s2_drop_footab_type: DROP TYPE public.footab;
+step s1_create_table_with_type: CREATE TABLE tabtype(a footab); <waiting ...>
+step s2_commit: COMMIT;
+step s1_create_table_with_type: <... completed>
+ERROR:  dependent type was concurrently dropped
+
+starting permutation: s1_begin s1_create_server_with_fdw_wrapper s2_drop_fdw_wrapper s1_commit
+step s1_begin: BEGIN;
+step s1_create_server_with_fdw_wrapper: CREATE SERVER srv_fdw_wrapper FOREIGN DATA WRAPPER fdw_wrapper;
+step s2_drop_fdw_wrapper: DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_fdw_wrapper: <... completed>
+ERROR:  cannot drop foreign-data wrapper fdw_wrapper because other objects depend on it
+
+starting permutation: s2_begin s2_drop_fdw_wrapper s1_create_server_with_fdw_wrapper s2_commit
+step s2_begin: BEGIN;
+step s2_drop_fdw_wrapper: DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT;
+step s1_create_server_with_fdw_wrapper: CREATE SERVER srv_fdw_wrapper FOREIGN DATA WRAPPER fdw_wrapper; <waiting ...>
+step s2_commit: COMMIT;
+step s1_create_server_with_fdw_wrapper: <... completed>
+ERROR:  dependent foreign-data wrapper was concurrently dropped
+
+starting permutation: s1_begin s1_alter_function_owner s2_drop_role s1_commit
+step s1_begin: BEGIN;
+step s1_alter_function_owner: ALTER FUNCTION public.falter() OWNER TO regress_dependency;
+step s2_drop_role: DROP ROLE regress_dependency; <waiting ...>
+step s1_commit: COMMIT;
+step s2_drop_role: <... completed>
+ERROR:  role "regress_dependency" cannot be dropped because some objects depend on it
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 1578ba191c8..83f626d51b5 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -126,3 +126,4 @@ test: serializable-parallel-3
 test: matview-write-skew
 test: lock-nowait
 test: for-portion-of
+test: test_dependencies_locks
\ No newline at end of file
diff --git a/src/test/isolation/specs/test_dependencies_locks.spec b/src/test/isolation/specs/test_dependencies_locks.spec
new file mode 100644
index 00000000000..fd80c9cb9b7
--- /dev/null
+++ b/src/test/isolation/specs/test_dependencies_locks.spec
@@ -0,0 +1,108 @@
+# Test that concurrent DDL properly prevents orphaned dependencies.
+#
+# When session 1 creates an object that depends on a referenced object,
+# and session 2 concurrently drops that referenced object, the lock
+# acquired during dependency recording must prevent the drop or the
+# create must fail with "dependent object does not exist".
+
+setup
+{
+	CREATE SCHEMA testschema;
+	CREATE SCHEMA alterschema;
+	CREATE TYPE public.foo as enum ('one', 'two');
+	CREATE TYPE public.footab as enum ('three', 'four');
+	CREATE DOMAIN id AS int;
+	CREATE FUNCTION f() RETURNS int LANGUAGE SQL RETURN 1;
+	CREATE FUNCTION public.falter() RETURNS int LANGUAGE SQL RETURN 1;
+	CREATE FOREIGN DATA WRAPPER fdw_wrapper;
+	CREATE ROLE regress_dependency;
+}
+
+teardown
+{
+	DROP FUNCTION IF EXISTS testschema.foo();
+	DROP FUNCTION IF EXISTS fooargtype(num foo);
+	DROP FUNCTION IF EXISTS footrettype();
+	DROP FUNCTION IF EXISTS foofunc();
+	DROP FUNCTION IF EXISTS public.falter();
+	DROP FUNCTION IF EXISTS alterschema.falter();
+	DROP DOMAIN IF EXISTS idid;
+	DROP SERVER IF EXISTS srv_fdw_wrapper;
+	DROP TABLE IF EXISTS tabtype;
+	DROP SCHEMA IF EXISTS testschema;
+	DROP SCHEMA IF EXISTS alterschema;
+	DROP TYPE IF EXISTS public.foo;
+	DROP TYPE IF EXISTS public.footab;
+	DROP DOMAIN IF EXISTS id;
+	DROP FUNCTION IF EXISTS f();
+	DROP FOREIGN DATA WRAPPER IF EXISTS fdw_wrapper;
+	DROP ROLE regress_dependency;
+}
+
+session "s1"
+
+step "s1_begin" { BEGIN; }
+step "s1_create_function_in_schema" { CREATE FUNCTION testschema.foo() RETURNS int AS 'select 1' LANGUAGE sql; }
+step "s1_create_function_with_argtype" { CREATE FUNCTION fooargtype(num foo) RETURNS int AS 'select 1' LANGUAGE sql; }
+step "s1_create_function_with_rettype" { CREATE FUNCTION footrettype() RETURNS id LANGUAGE sql RETURN 1; }
+step "s1_create_function_with_function" { CREATE FUNCTION foofunc() RETURNS int LANGUAGE SQL RETURN f() + 1; }
+step "s1_alter_function_owner" { ALTER FUNCTION public.falter() OWNER TO regress_dependency; }
+step "s1_alter_function_schema" { ALTER FUNCTION public.falter() SET SCHEMA alterschema; }
+step "s1_create_domain_with_domain" { CREATE DOMAIN idid as id; }
+step "s1_create_table_with_type" { CREATE TABLE tabtype(a footab); }
+step "s1_create_server_with_fdw_wrapper" { CREATE SERVER srv_fdw_wrapper FOREIGN DATA WRAPPER fdw_wrapper; }
+step "s1_commit" { COMMIT; }
+
+session "s2"
+
+step "s2_begin" { BEGIN; }
+step "s2_drop_schema" { DROP SCHEMA testschema; }
+step "s2_drop_alterschema" { DROP SCHEMA alterschema; }
+step "s2_drop_foo_type" { DROP TYPE public.foo; }
+step "s2_drop_foo_rettype" { DROP DOMAIN id; }
+step "s2_drop_footab_type" { DROP TYPE public.footab; }
+step "s2_drop_function_f" { DROP FUNCTION f(); }
+step "s2_drop_domain_id" { DROP DOMAIN id; }
+step "s2_drop_fdw_wrapper" { DROP FOREIGN DATA WRAPPER fdw_wrapper RESTRICT; }
+step "s2_drop_role" { DROP ROLE regress_dependency; }
+step "s2_commit" { COMMIT; }
+
+# function - schema
+permutation "s1_begin" "s1_create_function_in_schema" "s2_drop_schema" "s1_commit"
+permutation "s2_begin" "s2_drop_schema" "s1_create_function_in_schema" "s2_commit"
+
+# alter function - schema
+permutation "s1_begin" "s1_alter_function_schema" "s2_drop_alterschema" "s1_commit"
+permutation "s2_begin" "s2_drop_alterschema" "s1_alter_function_schema" "s2_commit"
+
+# function - argtype
+permutation "s1_begin" "s1_create_function_with_argtype" "s2_drop_foo_type" "s1_commit"
+permutation "s2_begin" "s2_drop_foo_type" "s1_create_function_with_argtype" "s2_commit"
+
+# function - rettype
+permutation "s1_begin" "s1_create_function_with_rettype" "s2_drop_foo_rettype" "s1_commit"
+permutation "s2_begin" "s2_drop_foo_rettype" "s1_create_function_with_rettype" "s2_commit"
+
+# function - function
+permutation "s1_begin" "s1_create_function_with_function" "s2_drop_function_f" "s1_commit"
+permutation "s2_begin" "s2_drop_function_f" "s1_create_function_with_function" "s2_commit"
+
+# domain - domain
+permutation "s1_begin" "s1_create_domain_with_domain" "s2_drop_domain_id" "s1_commit"
+permutation "s2_begin" "s2_drop_domain_id" "s1_create_domain_with_domain" "s2_commit"
+
+# table - type
+permutation "s1_begin" "s1_create_table_with_type" "s2_drop_footab_type" "s1_commit"
+permutation "s2_begin" "s2_drop_footab_type" "s1_create_table_with_type" "s2_commit"
+
+# server - foreign data wrapper
+permutation "s1_begin" "s1_create_server_with_fdw_wrapper" "s2_drop_fdw_wrapper" "s1_commit"
+permutation "s2_begin" "s2_drop_fdw_wrapper" "s1_create_server_with_fdw_wrapper" "s2_commit"
+
+# function - role
+permutation "s1_begin" "s1_alter_function_owner" "s2_drop_role" "s1_commit"
+
+# XXX: This permutation is disabled because the error message, "role
+# <OID> was concurrently dropped", contains an OID that is not stable.
+#
+# permutation "s2_begin" "s2_drop_role" "s1_alter_function_owner" "s2_commit"
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 6dd22be0e8d..b891d68d4a7 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2949,11 +2949,12 @@ begin;
 alter table alterlock2
 add constraint alterlock2nv foreign key (f1) references alterlock (f1) NOT VALID;
 select * from my_locks order by 1;
-  relname   |     max_lockmode      
-------------+-----------------------
- alterlock  | ShareRowExclusiveLock
- alterlock2 | ShareRowExclusiveLock
-(2 rows)
+    relname     |     max_lockmode      
+----------------+-----------------------
+ alterlock      | ShareRowExclusiveLock
+ alterlock2     | ShareRowExclusiveLock
+ alterlock_pkey | AccessShareLock
+(3 rows)
 
 commit;
 begin;
-- 
2.47.3

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 26 May 2026 20:18:50 +0000

Hi,

On Tue, May 26, 2026 at 09:00:11PM +0300, Heikki Linnakangas wrote:
> I had a closer look at patch 0001.

Thanks!

> It doesn't fix all the problems, we
> think it makes sense to commit it independently. 

Yeah, this will prevent orphaned objects which is a good step forward.

> So I'm focusing on that

Thanks! I'll resume working on something similar to 0002 and 0003 once 0001 gets
in.

> dependencies, in shdepLockAndCheckObject(). I see that you even copied the
> function comment from shdepLockAndCheckObject() to LockNotPinnedObject(),
> but I didn't see it being otherwise mentioned in this thread. In any case,
> that's a good argument for doing the same for non-shared dependencies that
> we already do for shared dependencies.

Right.

> Hmm, shouldn't that re-check that the relation exists, after acquiring the
> lock, like the non-relation codepath does? Otherwise it's a little pointless
> to acquire the lock.

Good catch! Your change:

Looks good to me.

> I did a bunch of little refactorings and ended up with the attached. Notable
> changes:
> 

Makes sense to me.

> - Removed ObjectByIdExist(), inlined it into the caller. The argument to use
> SnapshotSelf or not seemed a bit too special to be generally useful

Yeah, better to have this logic inlined as it will probably not be useful outside
of it.

> - Changed the error message and code to rhyme with the existing "role <OID>
> was concurrently dropped" errors. I didn't add the OID to the error message
> however, because that makes the testing hard.

Agreed that's better.

> shdepLockAndCheckObject() function. It's currently disabled because it

Nit: It also adds:

+# function - role

That is not disabled and would already pass without the changes added in 0001.

So I wonder if we could just start by adding this new test (and the XXX one) in
a dedicated patch and then add 0001 that would focus on orphaned stuff only.

A few comments:

1/

That doesn't look needed.

2/

It looks like pgindent "complains" on pg_depend.c for dependencyLockAndCheckObject().

Regards,

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 27 May 2026 18:53:35 +0300

> a dedicated patch and then add 0001 that would focus on orphaned stuff only.

True, we could do that. It doesn't really seem to me any better or worse 
than just including it in the same commit though.

> It looks like pgindent "complains" on pg_depend.c for dependencyLockAndCheckObject().

Fixed these, and did some more copy-editing on the comments. With that, 
committed and backpatched. Version 14 needed a little more work to 
backport because it didn't have IsPinnedObject(). And I noticed that 
recordDependencyFor() was being called with InvalidOid on ALTER TABLE 
commands on tables with dropped columns. That was bogus but harmless 
before this patch; I fixed that too.

Thanks!

- Heikki

---

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 1 Jun 2026 09:21:39 +0000

Hi,

On Wed, May 27, 2026 at 06:53:35PM +0300, Heikki Linnakangas wrote:
> committed and backpatched.

Thanks!

Now that we avoid orphaned objects dependencies, I resumed working on Robert's
concern about the TOCTOU window where a REVOKE could land between the original
permission check and the dependency recording.

Based on our discussion during PGConf.dev, PFA a new patch that uses the same
approach as RangeVarGetRelidExtended(): record SharedInvalidMessageCounter at the
time of the original aclcheck, then before locking compare the current counter to
the saved value. If it changed, recheck permission before acquiring the lock.
After the lock wait, if more invalidations arrived, release and retry.

Remarks:

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.

The patch adds a test that would fail without the TOCTOU protection in place.

Alternatives considered:

- To avoid allocating memory for each statement, keep the array in
TopMemoryContext and never free it (only resetting the count). But that left the
high-water mark allocated for the lifetime of the backend.

- Passing privilege info (roleId, mode) as extra arguments through the
dependency recording APIs (recordDependencyOn, recordMultipleDependencies,
etc.). It was discarded because expression-based dependencies (recordDependencyOnExpr,
find_expr_references_walker) discover objects by walking expression trees: the
caller never sees individual objects and cannot attach privilege info to them.

That's why I believe the tracking approach that is in the attached sounds like
a right approach. Bonus point, it would also help if we want to "ensure" that
we always do the acl check prior the dependency recording: that would avoid
cases like the one mentioned in [1] (for example, when we can create a view and
then record a dependency based on a function we don't have exec privilege on).

[1]: https://postgr.es/m/agsCqlLTytZCudMv%40bdtpg

Regards,

---

From: Jeff Davis <[email-masked]>
Date: Wed, 03 Jun 2026 11:08:13 -0700

> After the lock wait, if more invalidations arrived, release and

RangeVarGetRelidExtended() coordinates three things:

  - name lookup
  - lock
  - ACL check

whereas recheckAclAndLock() only coordinates the latter two. That means
there can still be some strange failures, like:

  -- Session 1
  BEGIN;
  DROP SCHEMA s2;

  -- Session 2
  SET search_path=s2, s1;
  CREATE FUNCTION f() RETURNS INT LANGUAGE plpgsql AS
    $$ BEGIN RETURN 42; END; $$;

  -- Session 1
  COMMIT;

  -- Session 2
  ERROR:  referenced schema was concurrently dropped

even though no schema was actually referenced in the query, and a retry
of the transaction successfully creates the function in s1. Is that
expected?

Regards,
	Jeff Davis

---

From: Jeff Davis <[email-masked]>
Date: Thu, 04 Jun 2026 10:03:22 -0700

> 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. XXX: consider refactoring so that we perform the name lookup,
acquire the lock, and check ACLs all in unison, like
RangeVarGetRelidExtended()."

Assuming I understand correctly.

Regards,
	Jeff Davis

---

From: Jeff Davis <[email-masked]>
Date: Thu, 04 Jun 2026 10:17:57 -0700

On Wed, 2026-06-03 at 11:08 -0700, Jeff Davis wrote:
> whereas recheckAclAndLock() only coordinates the latter two.

Given that we don't do another name lookup, the object Oid doesn't
change, and it's not obvious why we need a loop in this path.

A sequence like:

Earlier during DDL processing:
  0. Name lookup and ACL check (and track ACLs)

When recording dependencies:
  1. Lock object
  2. Check that it still exists, error if not
  3. recheck tracked ACLs, error if failure

could work too, right?

I see why you might want to do the checks while not holding the lock,
but it doesn't seem like a requirement (if the user doesn't have
permissions it should fail quickly and release). In any case, it's
worth a comment explaining why the loop is there.

Regards,
	Jeff Davis

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 5 Jun 2026 13:09:37 +0000

Hi,

On Thu, Jun 04, 2026 at 10:03:22AM -0700, Jeff Davis wrote:
> 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,

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,

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 5 Jun 2026 13:12:34 +0000

Hi,

>   3. recheck tracked ACLs, error if failure

Yeah, that's a good point, thanks!

> permissions it should fail quickly and release).

I agree that we can get rid of the loop and that's what v24 shared up-thread [1]
is now implementing. The downside is that one could take a lock on an object
he has no privileges on but then, as you said, it will be released shortly after.

[1]: https://postgr.es/m/aiLKkTC6QBt8i35P%40bdtpg

Regards,

---

From: Jeff Davis <[email-masked]>
Date: Mon, 08 Jun 2026 14:55:19 -0700

On Fri, 2026-06-05 at 13:09 +0000, Bertrand Drouvot wrote:
> the current patch

  /*
   * Look up a tracked aclcheck entry for the given object.
   * Returns true if found, filling in roleId, mode, and inval_count.
   * Searches from the end to find the most recent check (the one with
the
   * freshest inval_count).

What if two tracked ACL checks had different modes? Don't you need to
do both checks or somehow combine the modes in the tracking entry?

(I'm not sure if this is a practical problem or not.)

Regards,
	Jeff Davis

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 9 Jun 2026 06:47:06 +0000

Hi,

> (I'm not sure if this is a practical problem or not.)

Humm, good point. I was focusing on a "full" role revoke but maybe what you
describe could be a practical problem (in case of privilege revoke from the
role).

In order to be on the safe side of things, the attached now iterates through all
matching entries (and not only the last one). It's still a flat array as its
linear scan is O(n) in the number of tracked entries, but that's fine since a
typical DDL statement tracks only a few objects.

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 9 Jun 2026 18:04:20 +0000

Hi,

> typical DDL statement tracks only a few objects.

I just realized that the cfbot is complaining, let me fix it and come back with
an updated patch.

Regards,

---

From: Jeff Davis <[email-masked]>
Date: Tue, 09 Jun 2026 14:44:10 -0700

Hi,

On Tue, 2026-06-09 at 06:47 +0000, Bertrand Drouvot wrote:
> matching entries (and not only the last one).

I don't think:

  if (context == PROCESS_UTILITY_TOPLEVEL)

is quite right. With SPI, I bet you can end up in a situation where the
branch fires twice (causing an early reset) or not at all (because the
only call is PROCESS_UTLITY_QUERY).

In any case, I don't think mixing the tracking entries between entirely
different DDL commands is a good idea. If you execute DDL inside of SPI
in its own subtransaction, and it inserts a tracking entry to recheck
something, and then you roll back the subxact, you don't want the
tracking entry to then cause the outer transaction to fail. I didn't
test this, so if there's something preventing this kind of problem, let
me know.

We probably need to track where we are in the stack of ProcessUtility()
calls and keep the tracking entries separate, and always remove entries
from that level on return (or rollback).

Regards,
	Jeff Davis

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 10 Jun 2026 12:31:04 +0000

Hi,

On Tue, Jun 09, 2026 at 02:44:10PM -0700, Jeff Davis wrote:
> is quite right.

I think you are right. I changed this in the attached so that aclcheck_tracked_count
is saved on entry and restored on return of each ProcessUtility call, so that each
nesting level's entries are kept separate.

Also, the reset is now done whenever tracking is initialized (means at the
outermost ProcessUtility call that starts tracking).

I think that addresses your concerns and it also fixes the issues reported
by the cfbot (that I mentioned up-thread).

I also added more tests related to nested calls.

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 10 Jun 2026 15:16:56 +0000

Hi,

> I also added more tests related to nested calls.

PFA a new version of v26, it adds a new test as compared to the v26 previously
shared.

Regards,

---

From: Jeff Davis <[email-masked]>
Date: Mon, 15 Jun 2026 18:19:19 -0700

> PFA a new version of v26, it adds a new test as compared to the v26

I'd like to avoid adding lines to AbortTransaction(). Also, I think it
might miss subtransaction aborts, which could be relevant in complex
cases with SPI.

Can you use a structure like:

  ProcessUtility()
  {
      TrackAclTable *prevTrackAclTable = CurrentTrackAclTable;
      /* allocates in CurrentMemoryContext */
      CurrentTrackAclTable = NewTrackAclTable();

      PG_TRY();
      {
           ... rest of ProcessUtility ...
      }
      PG_FINALLY();
      {
          FreeTrackAclTable(CurrentTrackAclTable);
          CurrentTrackAclTable = prevTrackAclTable;
      }
      PG_END_TRY();
  }

That would avoid the need to create a special memory context; you could
just repalloc() the chunk allocated for the table. It would also mean
you don't have to track the stack frames manually with a counter, just
use a local variable.

Also, are you sure that the two call sites for aclcheck_track_record()
are enough? Or do we need checks in e.g. pg_attribute_aclcheck() as
well?

Regards,
	Jeff Davis

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 16 Jun 2026 10:09:48 +0000

Hi,

> you don't have to track the stack frames manually with a counter, just

The advantage of v26 is that it also checked for the outer while checking the
nested (that way a revoke that impacted the outer would fail sooner). That said,
I like your idea: it's more readable and probably less error-prone (don't use
global variable in multiple places and a dedicated counter for nested checks).

So, the attached implements it that way.

> are enough? Or do we need checks in e.g. pg_attribute_aclcheck() as

Currently, the only caller of pg_attribute_aclcheck_ext() that also records
dependencies on the checked object is checkFkeyPermissions(), which already
holds ShareRowExclusiveLock on the referenced table. While ShareRowExclusiveLock
prevents direct REVOKE on the table, it does not prevent role membership revokes.
So 0003 adds pg_attribute_aclcheck_ext to ACL tracking for dependency recording,
which also protects against new DDL callers.

The remaining aclcheck functions (pg_parameter_aclcheck and
pg_largeobject_aclcheck_snapshot) do not need tracking: pg_parameter_aclcheck
checks GUC parameters which are not dependency targets, and
pg_largeobject_aclcheck_snapshot is only called from inv_open() and
has_largeobject_privilege(), neither of which records dependencies.

That said, while looking at checkFkeyPermissions(), I realized that we have a
corner case in 0001.

Indeed, checkFkeyPermissions() first tries pg_class_aclcheck() (which fails for
column-level grants), then falls through to pg_attribute_aclcheck(). But as
aclcheck_track_record() is called for both successful and failed checks, then
the tracked failed entry could trigger a 'permission denied' in recheckAcl() if
catalog invalidations arrived between the check and dependency recording.

0002: fixes it by moving aclcheck_track_record() to after the permission check
succeeds in object_aclcheck_ext() and pg_class_aclcheck_ext(). Indeed, there is
no need to track failed permission checks. It also adds a test that reproduces
the issue by injecting invalidations while the FK creation is paused after the
failed table-level check. The test would fail without the fix in 0002 applied.

0003: adds pg_attribute_aclcheck_ext to ACL tracking for dependency recording.
It adds an attnum field to AclCheckEntry so that recheckAcl() can distinguish
column-level checks from table-level checks and call the appropriate recheck
function. InvalidAttrNumber means whole-object check. 

The sub-patches are to ease the review. They should probably be merged before
being pushed.

Regards,

---

From: Jeff Davis <[email-masked]>
Date: Tue, 16 Jun 2026 12:14:12 -0700

On Tue, 2026-06-16 at 10:09 +0000, Bertrand Drouvot wrote:
> no need to track failed permission checks.

IIUC, this is necessary for correctness. If an ACL failure doesn't
cause a transaction abort, then there's a danger that we cause the
transaction to fail that should have succeeded.

So the ACL tracking needs to be precise: we can't track an ACL check
unless a failure always causes transaction abort; and we must track an
ACL check if it would cause a transaction abort. Right?

Regards,
	Jeff Davis

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 17 Jun 2026 05:44:47 +0000

Hi,

> transaction to fail that should have succeeded.

Exactly, because we'd recheck an "harmless" failed ACL check and then produce
an error.

> ACL check if it would cause a transaction abort. Right?

I would say: we just need to track (and recheck) ACL checks that succeeded.

I think that there is no reason to recheck (and so to record) a failed ACL as what
we are dealing with here is the TOCTOU window. Re-checking a failed ACL check would
handle cases when a GRANT has been given during the TOCTOU window which is not
useful (for our protection goal) compared to re-checking a REVOKE during the
TOCTOU window (as the latter would record a dependency on an object we don't have
permission on).

Doing so, as proposed in 0002, allows us to fix the "re-check a harmless failed
ACL bug" (demonstrated by the added test) and still protect us for REVOKE during
the TOCTOU window.

Thoughts?

Regards,

---

From: Jeff Davis <[email-masked]>
Date: Thu, 18 Jun 2026 16:21:33 -0700

> I would say: we just need to track (and recheck) ACL checks that

IIUC, we cannot have false positives (tracking ACL checks that wouldn't
have caused an abort) nor can we have false negatives (missing an ACL
check that could cause an abort).

It's hard for me to convince myself that we got all the cases right;
and if we have, that they won't be broken in the future.

For instance, I just realized that something else I'm working on is
related:

https://www.postgresql.org/message-id/[email-masked]

It does an ACL check inside a subtransaction, where the parent
transaction is a DDL statement. It happens to be a DROP statement, so
it's not recording new dependencies, so I don't think it breaks your
tracking mechanism, but it's too close for comfort.

We could keep the transaction ID in the tracking record, and ignore
entries from an aborted subxact. But it's getting fairly complex and
delicate.

Thoughts?

Regards,
	Jeff Davis

---

From: Jeff Davis <[email-masked]>
Date: Thu, 18 Jun 2026 19:13:38 -0700

> check that could cause an abort).

Idea: what if we check for changes in ACLs on the object, rather than
whether it passes the check or not?

Then, if track an ACL check that wouldn't actually cause a failure,
then it still might be acceptable to throw an error if the ACL changes.
Still some details to sort out, so this is just an idea.

Regards,
	Jeff Davis

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 19 Jun 2026 13:28:21 +0000

Hi,

Right.

> tracking mechanism, but it's too close for comfort.

I see, I don't think we have this pattern currently but yeah we may have it
in the future (and our current tracking mechanism would probably fail in such a
case).

> entries from an aborted subxact. But it's getting fairly complex and

Agreed. We could use RegisterSubXactCallback to save/restore the entry count on
subtransaction abort, but as you say it adds complexity.

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 19 Jun 2026 13:36:45 +0000

Hi,

> Still some details to sort out, so this is just an idea.

Yeah, I think I do prefer this idea. As you say, that could cause an error even
if the ACL change does not REVOKE anything on this object (say the ACL change is
a GRANT), but that should be rare in practice and probably much simpler to reason
about that way.

But I don't think tracking ACL changes would be enough though. I think we would
also need to track ROLE changes.

So what about?

- Save a copy of the object's ACL and compare at recheck time: If not the same,
then error out.
- Save the ROLE membership and compare at recheck time. If not the same, then
error out.

That way we cover both parts: the object's ACL and the ROLE membership.

That's just a high level idea, I can move forward and try to implement it.

Thoughts?

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Thu, 25 Jun 2026 11:10:58 +0000

Hi,

> > entries from an aborted subxact. But it's getting fairly complex and

As discussed off-list, let's move forward with this proposal (as the save a copy
of the object's ACL + ROLE membership mentioned in [1] would be more complex and
fragile).

So PFA the new version doing so. It's the same patch series as the previous one
except that it adds:

0004: Discard ACL tracking entries from aborted subtransactions

It stores SubTransactionId in each AclCheckEntry at recording time.
On subtransaction abort, AtEOSubXact_AclTrack() discards entries whose subxid
is >= the aborting subtransaction's ID.

The code is pretty simple but I suspect that adding a test would probably need to
create a brand new module (say test_acltrack) with C code forcing
recordDependencyOn() and BeginInternalSubTransaction()) and I'm not sure it's
worth it. Indeed, I think the test would need to do things similar to what you
shared in [2].

The patch is split to ease the review but should probably be merged as a single
patch.

[1]: https://postgr.es/m/ajVF7YWf%2Bpxs4cOf%40bdtpg
[2]: https://postgr.es/m/[email-masked]

Regards,