On 2022-12-07 We 23:13, Nathan Bossart wrote: > On Wed, Dec 07, 2022 at 08:25:59PM -0600, Justin Pryzby wrote: >> Your patch makes it inconsistent with vacuum full, which is strange >> because vacuum full calls cluster. >> >> postgres=> VACUUM FULL t; >> VACUUM >> postgres=> CLUSTER t; >> ERROR: must be owner of table t > This is the existing behavior on HEAD. I think it has been this way for a > while. Granted, that doesn't mean it's ideal, but AFAICT it's intentional. > > Looking closer, the database ownership check in > get_tables_to_cluster_partitioned() appears to have no meaningful effect. > In this code path, cluster_rel() will always be called with CLUOPT_RECHECK, > and that function only checks for table ownership. Anything gathered in > get_tables_to_cluster_partitioned() that the user doesn't own will be > skipped. So, I don't think my patch changes the behavior in any meaningful > way, but I still think it's worthwhile to make the checks consistent.
We should probably talk about what the privileges should be, though. I think there's a case to be made that CLUSTER should be governed by the VACUUM privileges, given how VACUUM FULL is now implemented. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
