On Thu, Jun 15, 2023 at 09:46:33AM +0900, Michael Paquier wrote: > The result after 0001 is applied is that a couple of > object_ownercheck() calls that existed before ff9618e are removed from > some ACL checks in the REINDEX, CLUSTER and VACUUM paths. Is that OK > for shared relations and shouldn't cluster_is_permitted_for_relation() > include that? vacuum_is_permitted_for_relation() is consistent on > this side.
These object_ownercheck() calls were removed because they were redundant, as owners have all privileges by default. Privileges can be revoked from the owner, so an extra ownership check would effectively bypass the relation's ACL in that case. I looked around and didn't see any other examples of a combined ownership and ACL check like we were doing for MAINTAIN. The only thing that gives me pause is that the docs call out ownership as sufficient for some maintenance commands. With these patches, that's only true as long as no one revokes privileges from the owner. IMO we should update the docs and leave out the ownership checks since MAINTAIN is now a grantable privilege like any other. WDYT? -- Nathan Bossart Amazon Web Services: https://aws.amazon.com