0001-Improve-objectNamesToOids-comment.patch

text/plain

Filename: 0001-Improve-objectNamesToOids-comment.patch
Type: text/plain
Part: 0
Message: Re: Proper object locking for GRANT/REVOKE

Patch

Format: format-patch
Series: patch 0001
Subject: Improve objectNamesToOids() comment
File+
src/backend/catalog/aclchk.c 14 0
From 1cac7fecf80a7ed3dc4a714f7c984fd8934fc0ea Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 11 Jun 2025 17:08:23 +0200
Subject: [PATCH 1/2] Improve objectNamesToOids() comment

Commit d31bbfb6590 removed the comment at objectNamesToOids() that
there is no locking, because that commit added locking.  But to fix
all the problems, we'd still need a stronger lock.  So put the comment
back with more a detailed explanation.
---
 src/backend/catalog/aclchk.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 9ca8a88dc91..24948c1f05e 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -659,6 +659,20 @@ ExecGrantStmt_oids(InternalGrant *istmt)
  * objectNamesToOids
  *
  * Turn a list of object names of a given type into an Oid list.
+ *
+ * XXX This function intentionally takes only an AccessShareLock.  In the face
+ * of concurrent DDL, we might easily latch onto an old version of an object,
+ * causing the GRANT or REVOKE statement to fail.  But it does prevent the
+ * object from disappearing altogether.  To do better, we would need to use a
+ * self-exclusive lock, perhaps ShareUpdateExclusiveLock, here and before
+ * *every* CatalogTupleUpdate() of a row that GRANT/REVOKE can affect.
+ * Besides that additional work, this could have operational costs.  For
+ * example, it would make GRANT ALL TABLES IN SCHEMA terminate every
+ * autovacuum running in the schema and consume a shared lock table entry per
+ * table in the schema.  The user-visible benefit of that additional work is
+ * just changing "ERROR: tuple concurrently updated" to blocking.  That's not
+ * nothing, but it might not outweigh autovacuum termination and lock table
+ * consumption spikes.
  */
 static List *
 objectNamesToOids(ObjectType objtype, List *objnames, bool is_grant)

base-commit: 361499538c9d3640e1ed5522e08fdf81b08e76ae
-- 
2.49.0