Date: Sun, 24 Jul 2022 21:39:31 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: pgsql-hackers@postgresql.org
Subject: Re: predefined role(s) for VACUUM and ANALYZE
Message-ID: <20220725043931.GA4087907@nathanxps13>
References: <20220722203735.GB3996698@nathanxps13>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ReaqsoxgOBHFXBhH"
Content-Disposition: inline
In-Reply-To: <20220722203735.GB3996698@nathanxps13>
Archived-At: 
 <https://www.postgresql.org/message-id/20220725043931.GA4087907%40nathanxps13>
Precedence: bulk


--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Jul 22, 2022 at 01:37:35PM -0700, Nathan Bossart wrote:
> The attached patch adds a pg_vacuum_analyze role that allows VACUUM and
> ANALYZE commands on all relations.  I started by trying to introduce
> separate pg_vacuum and pg_analyze roles, but that quickly became
> complicated because the VACUUM and ANALYZE code is intertwined.  To
> initiate the discussion, here's the simplest thing I could think of.

And here's the same patch, but with docs that actually build.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

--ReaqsoxgOBHFXBhH
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="pg_vacuum_analyze_v2.patch"

diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index b968f740cb..203b713a4e 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -148,11 +148,14 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
   <title>Notes</title>
 
   <para>
-   To analyze a table, one must ordinarily be the table's owner or a
-   superuser.  However, database owners are allowed to
+   To analyze a table, one must ordinarily be the table's owner, a superuser, or
+   a role with privileges of the
+   <link linkend="predefined-roles-table"><literal>pg_vacuum_analyze</literal></link>
+   role.  However, database owners are allowed to
    analyze all tables in their databases, except shared catalogs.
    (The restriction for shared catalogs means that a true database-wide
-   <command>ANALYZE</command> can only be performed by a superuser.)
+   <command>ANALYZE</command> can only be performed by superusers and roles with
+   privileges of <literal>pg_vacuum_analyze</literal>.)
    <command>ANALYZE</command> will skip over any tables that the calling user
    does not have permission to analyze.
   </para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index c582021d29..12d7b96fee 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -356,11 +356,14 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
   <title>Notes</title>
 
    <para>
-    To vacuum a table, one must ordinarily be the table's owner or a
-    superuser.  However, database owners are allowed to
+    To vacuum a table, one must ordinarily be the table's owner, a superuser, or
+    a role with privileges of the
+    <link linkend="predefined-roles-table"><literal>pg_vacuum_analyze</literal></link>
+    role.  However, database owners are allowed to
     vacuum all tables in their databases, except shared catalogs.
     (The restriction for shared catalogs means that a true database-wide
-    <command>VACUUM</command> can only be performed by a superuser.)
+    <command>VACUUM</command> can only be performed by superusers and roles with
+    privileges of <literal>pg_vacuum_analyze</literal>.)
     <command>VACUUM</command> will skip over any tables that the calling user
     does not have permission to vacuum.
    </para>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 6eaaaa36b8..6052bd0c4f 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -588,6 +588,13 @@ DROP ROLE doomed_role;
        the <link linkend="sql-checkpoint"><command>CHECKPOINT</command></link>
        command.</entry>
       </row>
+      <row>
+       <entry>pg_vacuum_analyze</entry>
+       <entry>Allow executing the
+       <link linkend="sql-vacuum"><command>VACUUM</command></link> and
+       <link linkend="sql-analyze"><command>ANALYZE</command></link>
+       commands on all tables.</entry>
+      </row>
      </tbody>
     </tgroup>
    </table>
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 8df25f59d8..b3eb41a8cc 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -36,6 +36,7 @@
 #include "access/xact.h"
 #include "catalog/namespace.h"
 #include "catalog/index.h"
+#include "catalog/pg_authid.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
 #include "catalog/pg_namespace.h"
@@ -574,7 +575,8 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, bits32 options)
 	 * trying to vacuum or analyze the rest of the DB --- is this appropriate?
 	 */
 	if (pg_class_ownercheck(relid, GetUserId()) ||
-		(pg_database_ownercheck(MyDatabaseId, GetUserId()) && !reltuple->relisshared))
+		(pg_database_ownercheck(MyDatabaseId, GetUserId()) && !reltuple->relisshared) ||
+		has_privs_of_role(GetUserId(), ROLE_PG_VACUUM_ANALYZE))
 		return true;
 
 	relname = NameStr(reltuple->relname);
@@ -583,11 +585,14 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, bits32 options)
 	{
 		if (reltuple->relisshared)
 			ereport(WARNING,
-					(errmsg("skipping \"%s\" --- only superuser can vacuum it",
+					(errmsg("skipping \"%s\" --- only superusers and roles with "
+							"privileges of pg_vacuum_analyze can vacuum it",
 							relname)));
 		else if (reltuple->relnamespace == PG_CATALOG_NAMESPACE)
 			ereport(WARNING,
-					(errmsg("skipping \"%s\" --- only superuser or database owner can vacuum it",
+					(errmsg("skipping \"%s\" --- only superusers, roles with "
+							"privileges of pg_vacuum_analyze, or the database "
+							"owner can vacuum it",
 							relname)));
 		else
 			ereport(WARNING,
@@ -606,11 +611,14 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, bits32 options)
 	{
 		if (reltuple->relisshared)
 			ereport(WARNING,
-					(errmsg("skipping \"%s\" --- only superuser can analyze it",
+					(errmsg("skipping \"%s\" --- only superusers and roles with "
+							"privileges of pg_vacuum_analyze can analyze it",
 							relname)));
 		else if (reltuple->relnamespace == PG_CATALOG_NAMESPACE)
 			ereport(WARNING,
-					(errmsg("skipping \"%s\" --- only superuser or database owner can analyze it",
+					(errmsg("skipping \"%s\" --- only superusers, roles with "
+							"privileges of pg_vacuum_analyze, or the database "
+							"owner can analyze it",
 							relname)));
 		else
 			ereport(WARNING,
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index 3343a69ddb..f067fe1c57 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -84,5 +84,10 @@
   rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
   rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
   rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '4549', oid_symbol => 'ROLE_PG_VACUUM_ANALYZE',
+  rolname => 'pg_vacuum_analyze', rolsuper => 'f', rolinherit => 't',
+  rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+  rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+  rolpassword => '_null_', rolvaliduntil => '_null_' },
 
 ]
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index c63a157e5f..859be4c13e 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -302,18 +302,18 @@ VACUUM (ANALYZE) vacowned;
 WARNING:  skipping "vacowned" --- only table or database owner can vacuum it
 -- Catalog
 VACUUM pg_catalog.pg_class;
-WARNING:  skipping "pg_class" --- only superuser or database owner can vacuum it
+WARNING:  skipping "pg_class" --- only superusers, roles with privileges of pg_vacuum_analyze, or the database owner can vacuum it
 ANALYZE pg_catalog.pg_class;
-WARNING:  skipping "pg_class" --- only superuser or database owner can analyze it
+WARNING:  skipping "pg_class" --- only superusers, roles with privileges of pg_vacuum_analyze, or the database owner can analyze it
 VACUUM (ANALYZE) pg_catalog.pg_class;
-WARNING:  skipping "pg_class" --- only superuser or database owner can vacuum it
+WARNING:  skipping "pg_class" --- only superusers, roles with privileges of pg_vacuum_analyze, or the database owner can vacuum it
 -- Shared catalog
 VACUUM pg_catalog.pg_authid;
-WARNING:  skipping "pg_authid" --- only superuser can vacuum it
+WARNING:  skipping "pg_authid" --- only superusers and roles with privileges of pg_vacuum_analyze can vacuum it
 ANALYZE pg_catalog.pg_authid;
-WARNING:  skipping "pg_authid" --- only superuser can analyze it
+WARNING:  skipping "pg_authid" --- only superusers and roles with privileges of pg_vacuum_analyze can analyze it
 VACUUM (ANALYZE) pg_catalog.pg_authid;
-WARNING:  skipping "pg_authid" --- only superuser can vacuum it
+WARNING:  skipping "pg_authid" --- only superusers and roles with privileges of pg_vacuum_analyze can vacuum it
 -- Partitioned table and its partitions, nothing owned by other user.
 -- Relations are not listed in a single command to test ownership
 -- independently.

--ReaqsoxgOBHFXBhH--