Hi Nathan, On Fri, Jul 27, 2018 at 02:40:42PM +0000, Bossart, Nathan wrote: > I think I'm essentially suggesting what you have in 0002 but without > the new RangeVarGetRelidExtended() callback. I've attached a modified > version of 0002 that seems to fix the originally reported issue. (I > haven't looked into any extra handling needed for ANALYZE or > partitioned tables.) Running the same checks for all VACUUMs would > keep things simple and provide a more uniform user experience.
I have been looking at this patch for VACUUM (perhaps we ought to spawn a new thread as the cases of REINDEX and TRUNCATE have been addressed), and I can see a problem with this approach. If multiple transactions are used with a list of relations vacuum'ed then simply moving around the ACL checks would cause a new problem: if the relation vacuum'ed disappears when processing it with vacuum_rel() after the list of relations is built then we would get an ERROR instead of a skip because of pg_class_ownercheck() which is not fail-safe. Wouldn't it be enough to check just the ACLs in expand_vacuum_rel() and get_all_vacuum_rels()? The first is rather similar to what happens for TRUNCATE, and the second is similar to what has been fixed for VACUUM. We should also remove the relkind checks out of vacuum_skip_rel() as those checks are not completely consistent for all those code paths... -- Michael
signature.asc
Description: PGP signature
