Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qCAeI-0004ov-74 for pgsql-hackers@arkaria.postgresql.org; Thu, 22 Jun 2023 03:06:18 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1qCAeF-00029b-VO for pgsql-hackers@arkaria.postgresql.org; Thu, 22 Jun 2023 03:06:15 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qCAeF-00029I-Go for pgsql-hackers@lists.postgresql.org; Thu, 22 Jun 2023 03:06:15 +0000 Received: from mail-pf1-x430.google.com ([2607:f8b0:4864:20::430]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1qCAeB-003wFC-FC for pgsql-hackers@postgresql.org; Thu, 22 Jun 2023 03:06:14 +0000 Received: by mail-pf1-x430.google.com with SMTP id d2e1a72fcca58-666e6ecb52dso3253677b3a.2 for ; Wed, 21 Jun 2023 20:06:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687403169; x=1689995169; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Ynbm66ymF6lmJGyDeW+0hPTG+4QnL1gVkNJ0bHBOX44=; b=CS4mKMtHqO/UGIhz1Ewmft3SXcFSPJQo8YCEtDQM0nHno3xKskopOnmUbEuqLdIugL Xu70KPY7I2dAMxHjLR1Bbdsw4UrOw6xWL5y0EZafeB4Gpf/6ft34gL4Xoquic0wmpsj0 NeigCSpuhtS4bZ34BMKaw2+6xa8UY+PbGtIj8MfP2EKhBu1ZvWxEY+yUsCGlkfcvu2MV PQSRlZH8XZAPXigRWuSYRkwLKXWwxa0MrQk7C/2w2r4pUVf5MCsXR8LUbc8VlbuQqTTr +lxaitVGiyFZrtZirvyL9d8DJwcb+oY0eujvE8LXwQEqR1WtYD2SB1rHB5EQkuEnkkqC /zew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687403169; x=1689995169; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Ynbm66ymF6lmJGyDeW+0hPTG+4QnL1gVkNJ0bHBOX44=; b=RpNiugcwcs4Ai97of3LY0+G6w1jeS+Tsmx3emd7740qQBWEN1G7TQlc4cS1rY8J6z7 2Awm+M9pidCiiSc/ReDH19lH7DwnINYPk66IIBGilG16uHTLcNyTk4fHiTz6bah9hu/V PanSh2PCzEEeVr4Hq/Ub63NGJZfLV6uO62D3On9Y3SNPa3ItJsppOQjKHo5ntQYxnD9b IDXYgE8N93zz8CnJFCPeadyeeWEahR39LFKPx0ryTEFEiHUNZdLDNTtXpzn5yNZnHe9H LnQrquB1Or/0G8d7R2w8KPGUR1CFCYBi4ko5eZEqVIYYNC+1j4JJ3uBUZf5Yz7YQAldL 3o5g== X-Gm-Message-State: AC+VfDzaPeltRFlysFMHg2wnNVcbRhq2xnlbH0pIhPyLDfsQTe2L8ymI dhJ8xykdg5q+Y4HGb9EBzuM= X-Google-Smtp-Source: ACHHUZ6Toy7gZz4HdiI1pJnxZ61Gc+4/7DjHHr0AhPSA3ce6Gde7fEsreJbjQYcAJ7EzQ68LHKNciQ== X-Received: by 2002:aa7:888c:0:b0:63d:3339:e967 with SMTP id z12-20020aa7888c000000b0063d3339e967mr15199453pfe.19.1687403169334; Wed, 21 Jun 2023 20:06:09 -0700 (PDT) Received: from nathanxps13 ([50.47.162.83]) by smtp.gmail.com with ESMTPSA id e6-20020aa78c46000000b0065ecdefa57fsm3558715pfd.0.2023.06.21.20.06.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Jun 2023 20:06:08 -0700 (PDT) Date: Wed, 21 Jun 2023 20:06:06 -0700 From: Nathan Bossart To: Michael Paquier Cc: Jeff Davis , Ted Yu , Pavel Luzanov , Justin Pryzby , pgsql-hackers@postgresql.org Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX Message-ID: <20230622030606.GA1074545@nathanxps13> References: <20230615041044.GA736001@nathanxps13> <20230615235700.GA877311@nathanxps13> <20230616052025.GA1026700@nathanxps13> <20230619215534.GA442477@nathanxps13> <20230620225257.GA771663@nathanxps13> <20230621041518.GA774124@nathanxps13> <20230621171624.GA881253@nathanxps13> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="qMm9M+Fa2AknHoGS" Content-Disposition: inline In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --qMm9M+Fa2AknHoGS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Jun 22, 2023 at 10:46:41AM +0900, Michael Paquier wrote: > On Wed, Jun 21, 2023 at 10:16:24AM -0700, Nathan Bossart wrote: >>> I think that there is a testing gap with the coverage of CLUSTER. >>> "Ownership of partitions is checked" is a test that looks for the case >>> where regress_ptnowner owns the partitioned table and one of its >>> partitions, checking that the leaf not owned is skipped, but we don't >>> have a test where we attempt a CLUSTER on the partitioned table with >>> regress_ptnowner *not* owning the partitioned table, only one or more >>> of its partitions owned by regress_ptnowner. In this case, the >>> command would fail. >> >> We could add something for this, but it'd really just exercise the checks >> in RangeVarCallbackMaintainsTable(), which already has a decent amount of >> coverage. > > It seems to me that this has some value for the CLUSTER path, so I > would add a small thing for it. Done. > - /* > - * We already checked that the user has privileges to CLUSTER the > - * partitioned table when we locked it earlier, so there's no need to > - * check the privileges again here. > - */ > + if (!cluster_is_permitted_for_relation(relid, GetUserId())) > + continue; > I would add a comment here that this ACL recheck for the leaves is an > important thing to keep around as it impacts the case where the leaves > have a different owner than the parent, and the owner of the parent > clusters it. The only place in the tests where this has an influence > is the isolation test cluster-conflict-partition. Done. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com --qMm9M+Fa2AknHoGS Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0001-partial-revert-of-ff9618e82a.patch" From 941371500f66690bbf200dfb555cc17d947cf0d1 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 19 Jun 2023 13:57:57 -0700 Subject: [PATCH v9 1/2] partial revert of ff9618e82a --- doc/src/sgml/ref/analyze.sgml | 5 +-- doc/src/sgml/ref/cluster.sgml | 5 +-- doc/src/sgml/ref/lock.sgml | 5 +-- doc/src/sgml/ref/reindex.sgml | 6 +--- doc/src/sgml/ref/vacuum.sgml | 5 +-- src/backend/commands/cluster.c | 12 ++++--- src/backend/commands/indexcmds.c | 27 +++++++-------- src/backend/commands/lockcmds.c | 8 ----- src/backend/commands/tablecmds.c | 34 +++---------------- src/backend/commands/vacuum.c | 10 ++---- src/include/commands/tablecmds.h | 1 - .../expected/cluster-conflict-partition.out | 14 ++++---- .../specs/cluster-conflict-partition.spec | 7 ++-- src/test/regress/expected/cluster.out | 7 +++- src/test/regress/expected/create_index.out | 4 +-- src/test/regress/expected/privileges.out | 8 ++--- src/test/regress/expected/vacuum.out | 18 ++++++++++ src/test/regress/sql/cluster.sql | 3 ++ 18 files changed, 74 insertions(+), 105 deletions(-) diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml index 20c6f9939f..30a893230e 100644 --- a/doc/src/sgml/ref/analyze.sgml +++ b/doc/src/sgml/ref/analyze.sgml @@ -190,10 +190,7 @@ ANALYZE [ VERBOSE ] [ table_and_columnsANALYZE can only be performed by superusers and roles - with privileges of pg_maintain.) If a role has - permission to ANALYZE a partitioned table, it is also - permitted to ANALYZE each of its partitions, regardless - of whether the role has the aforementioned privileges on the partition. + with privileges of pg_maintain.) ANALYZE will skip over any tables that the calling user does not have permission to analyze. diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 29f0f1fd90..f0dd7faed5 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -137,10 +137,7 @@ CLUSTER [VERBOSE] on the table or be the table's owner, a superuser, or a role with privileges of the pg_maintain - role. If a role has permission to CLUSTER a partitioned - table, it is also permitted to CLUSTER each of its - partitions, regardless of whether the role has the aforementioned - privileges on the partition. CLUSTER will skip over any + role. CLUSTER will skip over any tables that the calling user does not have permission to cluster. diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml index 5b3b2b793a..8524182211 100644 --- a/doc/src/sgml/ref/lock.sgml +++ b/doc/src/sgml/ref/lock.sgml @@ -177,10 +177,7 @@ LOCK [ TABLE ] [ ONLY ] name [ * ] MODE (or a less-conflicting mode as described in ) is permitted. If a user has SELECT privileges on the table, ACCESS SHARE - MODE is permitted. If a role has permission to lock a - partitioned table, it is also permitted to lock each of its partitions, - regardless of whether the role has the aforementioned privileges on the - partition. + MODE is permitted. diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 71455dfdc7..23f8c7630b 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -306,11 +306,7 @@ REINDEX [ ( option [, ...] ) ] { DA indexes on shared catalogs will be skipped unless the user owns the catalog (which typically won't be the case), has privileges of the pg_maintain role, or has the MAINTAIN - privilege on the catalog. If a role has permission to - REINDEX a partitioned table, it is also permitted to - REINDEX each of its partitions, regardless of whether the - role has the aforementioned privileges on the partition. Of course, - superusers can always reindex anything. + privilege on the catalog. Of course, superusers can always reindex anything. diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 57bc4c23ec..445325e14c 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -452,10 +452,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ relname))); /* Check permissions */ - if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK && - !has_partition_ancestor_privs(relId, GetUserId(), ACL_MAINTAIN)) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE, - relation->relname); -} - -/* - * If relid is a partition, returns whether userid has any of the privileges - * specified in acl on any of its ancestors. Otherwise, returns false. - */ -bool -has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl) -{ - List *ancestors; - ListCell *lc; - - if (!get_rel_relispartition(relid)) - return false; - - ancestors = get_partition_ancestors(relid); - foreach(lc, ancestors) - { - Oid ancestor = lfirst_oid(lc); - - if (OidIsValid(ancestor) && - pg_class_aclcheck(ancestor, userid, acl) == ACLCHECK_OK) - return true; - } - - return false; + aclresult = pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_TABLE, relation->relname); } /* diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index bb79de4da6..7fe6a54c06 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -41,7 +41,6 @@ #include "catalog/pg_namespace.h" #include "commands/cluster.h" #include "commands/defrem.h" -#include "commands/tablecmds.h" #include "commands/vacuum.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -721,17 +720,12 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple, /*---------- * A role has privileges to vacuum or analyze the relation if any of the * following are true: - * - the role is a superuser - * - the role owns the relation * - the role owns the current database and the relation is not shared - * - the role has been granted the MAINTAIN privilege on the relation - * - the role has privileges to vacuum/analyze any of the relation's - * partition ancestors + * - the role has the MAINTAIN privilege on the relation *---------- */ if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) || - pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK || - has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN)) + pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK) return true; relname = NameStr(reltuple->relname); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 17b9404937..250d89ff88 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -99,7 +99,6 @@ extern void AtEOSubXact_on_commit_actions(bool isCommit, extern void RangeVarCallbackMaintainsTable(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); -extern bool has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl); extern void RangeVarCallbackOwnsRelation(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); diff --git a/src/test/isolation/expected/cluster-conflict-partition.out b/src/test/isolation/expected/cluster-conflict-partition.out index 8d21276996..7be9e56ef1 100644 --- a/src/test/isolation/expected/cluster-conflict-partition.out +++ b/src/test/isolation/expected/cluster-conflict-partition.out @@ -3,7 +3,7 @@ Parsed test spec with 2 sessions starting permutation: s1_begin s1_lock_parent s2_auth s2_cluster s1_commit s2_reset step s1_begin: BEGIN; step s1_lock_parent: LOCK cluster_part_tab IN SHARE UPDATE EXCLUSIVE MODE; -step s2_auth: SET ROLE regress_cluster_part; +step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR; step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; step s1_commit: COMMIT; step s2_cluster: <... completed> @@ -11,7 +11,7 @@ step s2_reset: RESET ROLE; starting permutation: s1_begin s2_auth s1_lock_parent s2_cluster s1_commit s2_reset step s1_begin: BEGIN; -step s2_auth: SET ROLE regress_cluster_part; +step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR; step s1_lock_parent: LOCK cluster_part_tab IN SHARE UPDATE EXCLUSIVE MODE; step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; step s1_commit: COMMIT; @@ -21,17 +21,15 @@ step s2_reset: RESET ROLE; starting permutation: s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_reset step s1_begin: BEGIN; step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE; -step s2_auth: SET ROLE regress_cluster_part; -step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; +step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR; +step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; step s1_commit: COMMIT; -step s2_cluster: <... completed> step s2_reset: RESET ROLE; starting permutation: s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset step s1_begin: BEGIN; -step s2_auth: SET ROLE regress_cluster_part; +step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR; step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE; -step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; +step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; step s1_commit: COMMIT; -step s2_cluster: <... completed> step s2_reset: RESET ROLE; diff --git a/src/test/isolation/specs/cluster-conflict-partition.spec b/src/test/isolation/specs/cluster-conflict-partition.spec index ae38cb4ee3..4d38a7f49a 100644 --- a/src/test/isolation/specs/cluster-conflict-partition.spec +++ b/src/test/isolation/specs/cluster-conflict-partition.spec @@ -23,12 +23,15 @@ step s1_lock_child { LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE; step s1_commit { COMMIT; } session s2 -step s2_auth { SET ROLE regress_cluster_part; } +step s2_auth { SET ROLE regress_cluster_part; SET client_min_messages = ERROR; } step s2_cluster { CLUSTER cluster_part_tab USING cluster_part_ind; } step s2_reset { RESET ROLE; } -# CLUSTER waits if locked, passes for all cases. +# CLUSTER on the parent waits if locked, passes for all cases. permutation s1_begin s1_lock_parent s2_auth s2_cluster s1_commit s2_reset permutation s1_begin s2_auth s1_lock_parent s2_cluster s1_commit s2_reset + +# When taking a lock on a partition leaf, CLUSTER on the parent skips +# the leaf, passes for all cases. permutation s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_reset permutation s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 2eec483eaa..a13aafff0b 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -502,12 +502,17 @@ CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); CREATE ROLE regress_ptnowner; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; +SET SESSION AUTHORIZATION regress_ptnowner; +CLUSTER ptnowner USING ptnowner_i_idx; +ERROR: permission denied for table ptnowner +RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS SELECT oid, relname, relfilenode FROM pg_partition_tree('ptnowner') AS tree JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +WARNING: permission denied to cluster "ptnowner2", skipping it RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -515,7 +520,7 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a -----------+---------- ptnowner | t ptnowner1 | f - ptnowner2 | f + ptnowner2 | t (3 rows) DROP TABLE ptnowner; diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index acfd9d1f4f..1473bc3175 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2831,9 +2831,9 @@ RESET ROLE; GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; SET SESSION ROLE regress_reindexuser; REINDEX TABLE pg_toast.pg_toast_1260; -ERROR: must be owner of table pg_toast_1260 +ERROR: permission denied for table pg_toast_1260 REINDEX INDEX pg_toast.pg_toast_1260_index; -ERROR: must be owner of index pg_toast_1260_index +ERROR: permission denied for index pg_toast_1260_index -- Clean up RESET ROLE; REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 3cf4ac8c9e..3e4dfcc2ec 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -2928,13 +2928,13 @@ WARNING: permission denied to analyze "maintain_test", skipping it VACUUM (ANALYZE) maintain_test; WARNING: permission denied to vacuum "maintain_test", skipping it CLUSTER maintain_test USING maintain_test_a_idx; -ERROR: must be owner of table maintain_test +ERROR: permission denied for table maintain_test REFRESH MATERIALIZED VIEW refresh_test; -ERROR: must be owner of table refresh_test +ERROR: permission denied for table refresh_test REINDEX TABLE maintain_test; -ERROR: must be owner of table maintain_test +ERROR: permission denied for table maintain_test REINDEX INDEX maintain_test_a_idx; -ERROR: must be owner of index maintain_test_a_idx +ERROR: permission denied for index maintain_test_a_idx REINDEX SCHEMA reindex_test; ERROR: must be owner of schema reindex_test RESET ROLE; diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index 41e020cf20..4def90b805 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -442,14 +442,20 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum; ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; SET ROLE regress_vacuum; VACUUM vacowned_parted; +WARNING: permission denied to vacuum "vacowned_part2", skipping it VACUUM vacowned_part1; VACUUM vacowned_part2; +WARNING: permission denied to vacuum "vacowned_part2", skipping it ANALYZE vacowned_parted; +WARNING: permission denied to analyze "vacowned_part2", skipping it ANALYZE vacowned_part1; ANALYZE vacowned_part2; +WARNING: permission denied to analyze "vacowned_part2", skipping it VACUUM (ANALYZE) vacowned_parted; +WARNING: permission denied to vacuum "vacowned_part2", skipping it VACUUM (ANALYZE) vacowned_part1; VACUUM (ANALYZE) vacowned_part2; +WARNING: permission denied to vacuum "vacowned_part2", skipping it RESET ROLE; -- Only one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO CURRENT_USER; @@ -478,14 +484,26 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum; ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; +WARNING: permission denied to vacuum "vacowned_part1", skipping it +WARNING: permission denied to vacuum "vacowned_part2", skipping it VACUUM vacowned_part1; +WARNING: permission denied to vacuum "vacowned_part1", skipping it VACUUM vacowned_part2; +WARNING: permission denied to vacuum "vacowned_part2", skipping it ANALYZE vacowned_parted; +WARNING: permission denied to analyze "vacowned_part1", skipping it +WARNING: permission denied to analyze "vacowned_part2", skipping it ANALYZE vacowned_part1; +WARNING: permission denied to analyze "vacowned_part1", skipping it ANALYZE vacowned_part2; +WARNING: permission denied to analyze "vacowned_part2", skipping it VACUUM (ANALYZE) vacowned_parted; +WARNING: permission denied to vacuum "vacowned_part1", skipping it +WARNING: permission denied to vacuum "vacowned_part2", skipping it VACUUM (ANALYZE) vacowned_part1; +WARNING: permission denied to vacuum "vacowned_part1", skipping it VACUUM (ANALYZE) vacowned_part2; +WARNING: permission denied to vacuum "vacowned_part2", skipping it RESET ROLE; DROP TABLE vacowned; DROP TABLE vacowned_parted; diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index a4cfaae807..b7115f8610 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -238,6 +238,9 @@ CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); CREATE ROLE regress_ptnowner; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; +SET SESSION AUTHORIZATION regress_ptnowner; +CLUSTER ptnowner USING ptnowner_i_idx; +RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS SELECT oid, relname, relfilenode FROM pg_partition_tree('ptnowner') AS tree -- 2.25.1 --qMm9M+Fa2AknHoGS Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0002-simplify-privilege-related-documentation-for-main.patch" From 99b0179198080d556d402c614792ace465f94a61 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 19 Jun 2023 14:07:26 -0700 Subject: [PATCH v9 2/2] simplify privilege-related documentation for maintenance commands --- doc/src/sgml/ref/analyze.sgml | 8 +----- doc/src/sgml/ref/cluster.sgml | 6 +---- doc/src/sgml/ref/lock.sgml | 6 ++--- .../sgml/ref/refresh_materialized_view.sgml | 6 ++--- doc/src/sgml/ref/reindex.sgml | 26 +++++++++---------- doc/src/sgml/ref/vacuum.sgml | 8 +----- doc/src/sgml/user-manag.sgml | 3 ++- 7 files changed, 22 insertions(+), 41 deletions(-) diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml index 30a893230e..954491b5df 100644 --- a/doc/src/sgml/ref/analyze.sgml +++ b/doc/src/sgml/ref/analyze.sgml @@ -183,14 +183,8 @@ ANALYZE [ VERBOSE ] [ table_and_columns To analyze a table, one must ordinarily have the MAINTAIN - privilege on the table or be the table's owner, a superuser, or a role with - privileges of the - pg_maintain - role. However, database owners are allowed to + privilege on the table. 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 - ANALYZE can only be performed by superusers and roles - with privileges of pg_maintain.) ANALYZE will skip over any tables that the calling user does not have permission to analyze. diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index f0dd7faed5..06f3d269e6 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -134,11 +134,7 @@ CLUSTER [VERBOSE] To cluster a table, one must have the MAINTAIN privilege - on the table or be the table's owner, a superuser, or a role with - privileges of the - pg_maintain - role. CLUSTER will skip over any - tables that the calling user does not have permission to cluster. + on the table. diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml index 8524182211..070855da18 100644 --- a/doc/src/sgml/ref/lock.sgml +++ b/doc/src/sgml/ref/lock.sgml @@ -166,10 +166,8 @@ LOCK [ TABLE ] [ ONLY ] name [ * ] To lock a table, the user must have the right privilege for the specified - lockmode, or be the table's - owner, a superuser, or a role with privileges of the pg_maintain - role. If the user has MAINTAIN, + lockmode. + If the user has MAINTAIN, UPDATE, DELETE, or TRUNCATE privileges on the table, any lockmode is permitted. If the user has diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml index 4d79b6ae7f..19737668cd 100644 --- a/doc/src/sgml/ref/refresh_materialized_view.sgml +++ b/doc/src/sgml/ref/refresh_materialized_view.sgml @@ -31,10 +31,8 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] name REFRESH MATERIALIZED VIEW completely replaces the - contents of a materialized view. To execute this command you must be the - owner of the materialized view, have privileges of the - pg_maintain - role, or have the MAINTAIN + contents of a materialized view. To execute this command you must have the + MAINTAIN privilege on the materialized view. The old contents are discarded. If WITH DATA is specified (or defaults) the backing query is executed to provide the new data, and the materialized view is left in a diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 23f8c7630b..bef3486843 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -292,21 +292,21 @@ REINDEX [ ( option [, ...] ) ] { DA - Reindexing a single index or table requires being the owner of that - index or table, having privileges of the - pg_maintain - role, or having the MAINTAIN privilege on the - table. Reindexing a schema or database requires being the + Reindexing a single index or table requires + having the MAINTAIN privilege on the + table. Note that while REINDEX on a partitioned index or + table requires having the MAINTAIN privilege on the + partitioned table, such commands skip the privilege checks when processing + the individual partitions. Reindexing a schema or database requires being the owner of that schema or database or having privileges of the - pg_maintain role. Note specifically that it's thus + pg_maintain + role. Note specifically that it's thus possible for non-superusers to rebuild indexes of tables owned by - other users. However, as a special exception, when - REINDEX DATABASE, REINDEX SCHEMA - or REINDEX SYSTEM is issued by a non-superuser, - indexes on shared catalogs will be skipped unless the user owns the - catalog (which typically won't be the case), has privileges of the - pg_maintain role, or has the MAINTAIN - privilege on the catalog. Of course, superusers can always reindex anything. + other users. However, as a special exception, + REINDEX DATABASE, REINDEX SCHEMA, + and REINDEX SYSTEM will skip indexes on shared catalogs + unless the user has the MAINTAIN privilege on the + catalog. diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 445325e14c..c42bbea9e2 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -445,14 +445,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ pg_maintain - role. However, database owners are allowed to + privilege on the table. 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 - VACUUM can only be performed by superusers and roles - with privileges of pg_maintain.) VACUUM will skip over any tables that the calling user does not have permission to vacuum. diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index b6c37ccef2..e1540dd481 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -692,7 +692,8 @@ DROP ROLE doomed_role; REFRESH MATERIALIZED VIEW, REINDEX, and LOCK TABLE on all - relations. + relations, as if having MAINTAIN rights on those + objects, even without having it explicitly. pg_use_reserved_connections -- 2.25.1 --qMm9M+Fa2AknHoGS--