On Tue, Jun 30, 2026 at 01:47:41PM +0900, Michael Paquier wrote:
> Something that still feels off to me is to blindly use _ext() in
> vacuum_is_permitted_for_relation(), where we *may* already hold a lock
> on the relation whose ACL is checked.  In this case missing a relation
> is not fine, so this would make the code more brittle in the
> single-relation case under autovacuum or a VACUUM with a list of
> relations provided by a user.

Yeah, so we should only use it for get_all_vacuum_rels(), as in the
attached.

-- 
nathan
>From aad9a02072544a7bc2c3f861987748b815fcddc9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 30 Jun 2026 10:29:16 -0500
Subject: [PATCH v8 1/1] handle concurrent drop in database-wide vacuum

---
 src/backend/commands/analyze.c |  3 ++-
 src/backend/commands/vacuum.c  | 23 ++++++++++++++++++-----
 src/include/commands/vacuum.h  |  2 +-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index f66e80b757c..c28b9dae983 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -157,7 +157,8 @@ analyze_rel(Oid relid, RangeVar *relation,
         */
        if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
                                                                                
  onerel->rd_rel,
-                                                                               
  params->options & ~VACOPT_VACUUM))
+                                                                               
  params->options & ~VACOPT_VACUUM,
+                                                                               
  false))
        {
                relation_close(onerel, ShareUpdateExclusiveLock);
                return;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a4abb29cf64..a402d2330f0 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -718,9 +718,10 @@ vacuum(List *relations, const VacuumParams *params, 
BufferAccessStrategy bstrate
  */
 bool
 vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
-                                                                uint32 options)
+                                                                uint32 
options, bool missing_ok)
 {
        char       *relname;
+       bool            is_missing = false;
 
        Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
 
@@ -733,9 +734,20 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class 
reltuple,
         */
        if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) &&
                 !reltuple->relisshared) ||
-               pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == 
ACLCHECK_OK)
+               pg_class_aclcheck_ext(relid, GetUserId(), ACL_MAINTAIN,
+                                                         missing_ok ? 
&is_missing : NULL) == ACLCHECK_OK)
                return true;
 
+       /*
+        * If the relation was concurrently dropped, nothing to do.  Note that
+        * this is only reachable when the caller specified missing_ok.
+        */
+       if (is_missing)
+       {
+               Assert(missing_ok);
+               return false;
+       }
+
        relname = NameStr(reltuple->relname);
 
        if ((options & VACOPT_VACUUM) != 0)
@@ -956,7 +968,7 @@ expand_vacuum_rel(VacuumRelation *vrel, MemoryContext 
vac_context,
                 * Make a returnable VacuumRelation for this rel if the user 
has the
                 * required privileges.
                 */
-               if (vacuum_is_permitted_for_relation(relid, classForm, options))
+               if (vacuum_is_permitted_for_relation(relid, classForm, options, 
false))
                {
                        oldcontext = MemoryContextSwitchTo(vac_context);
                        vacrels = lappend(vacrels, 
makeVacuumRelation(vrel->relation,
@@ -1069,7 +1081,7 @@ get_all_vacuum_rels(MemoryContext vac_context, int 
options)
                        continue;
 
                /* check permissions of relation */
-               if (!vacuum_is_permitted_for_relation(relid, classForm, 
options))
+               if (!vacuum_is_permitted_for_relation(relid, classForm, 
options, true))
                        continue;
 
                /*
@@ -2115,7 +2127,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams 
params,
         */
        if (!vacuum_is_permitted_for_relation(priv_relid,
                                                                                
  rel->rd_rel,
-                                                                               
  params.options & ~VACOPT_ANALYZE))
+                                                                               
  params.options & ~VACOPT_ANALYZE,
+                                                                               
  false))
        {
                relation_close(rel, lmode);
                PopActiveSnapshot();
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 956d9cea36d..e62f23748dc 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -389,7 +389,7 @@ extern bool vacuum_xid_failsafe_check(const struct 
VacuumCutoffs *cutoffs);
 extern void vac_update_datfrozenxid(void);
 extern void vacuum_delay_point(bool is_analyze);
 extern bool vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
-                                                                               
         uint32 options);
+                                                                               
         uint32 options, bool missing_ok);
 extern Relation vacuum_open_relation(Oid relid, RangeVar *relation,
                                                                         uint32 
options, bool verbose,
                                                                         
LOCKMODE lmode);
-- 
2.50.1 (Apple Git-155)

Reply via email to