Hi hackers, While looking into other opportunities for per-table permissions, I noticed a weird discrepancy in CLUSTER. When evaluating whether the current user has permission to CLUSTER a table, we ordinarily just check for ownership. However, the database owner is also allowed to CLUSTER all partitions that are not shared. This was added in 3f19e17, and I didn't see any discussion about it in the corresponding thread [0].
My first instinct is that we should just remove the database ownership check, which is what I've done in the attached patch. I don't see any strong reason to complicate matters with special database-owner-but-not-shared checks like other commands (e.g., VACUUM). But perhaps we should do so just for consistency's sake. Thoughts? It was also noted elsewhere [1] that the privilege requirements for CLUSTER are not documented. The attached patch adds such documentation. [0] https://postgr.es/m/20220411140609.GF26620%40telsasoft.com [1] https://postgr.es/m/661148f4-c7f1-dec1-2bc8-29f3bd58e242%40postgrespro.ru -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index c37f4236f1..feb61e6ec2 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -67,7 +67,8 @@ CLUSTER [VERBOSE] </para> <para> - <command>CLUSTER</command> without any parameter reclusters all the + <command>CLUSTER</command> without a + <replaceable class="parameter">table_name</replaceable> reclusters all the previously-clustered tables in the current database that the calling user owns, or all such tables if called by a superuser. This form of <command>CLUSTER</command> cannot be executed inside a transaction @@ -132,6 +133,13 @@ CLUSTER [VERBOSE] <refsect1> <title>Notes</title> + <para> + To cluster a table, one must be the table's owner or a superuser. + Database-wide clusters and clusters on partitioned tables will silently + skip over any tables that the calling user does not have permission to + cluster. + </para> + <para> In cases where you are accessing single rows randomly within a table, the actual order of the data in the diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 07e091bb87..a8a51cc674 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1693,9 +1693,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) continue; /* Silently skip partitions which the user has no access to. */ - if (!object_ownercheck(RelationRelationId, relid, GetUserId()) && - (!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) || - IsSharedRelation(relid))) + if (!object_ownercheck(RelationRelationId, relid, GetUserId())) continue; /* Use a permanent memory context for the result list */