Date: Wed, 7 Dec 2022 20:13:13 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: fix and document CLUSTER privileges
Message-ID: <20221208041313.GA216874@nathanxps13>
References: <20221207223924.GA4182184@nathanxps13>
 <20221208022559.GA27893@telsasoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221208022559.GA27893@telsasoft.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20221208041313.GA216874%40nathanxps13>
Precedence: bulk

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.

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