Re: Improve behavior of concurrent ANALYZE/VACUUM

On Thu, Aug 16, 2018 at 03:07:18PM -0400, Robert Haas wrote:
> We definitely don't want to use RangeVarGetRelidExtended more than
> necessary. It is important that we use that function only when
> necessary - that is, to look up names supplied by users - and it is
> also important that we look up each user-supplied name only once, lest
> we get different answers on different occasions, possibly introducing
> either outright security problems or at the least ludicrous behavior.

Thanks, that matches my feelings about this stuff.

> In the case where we have an OID already, I think we could just
> perform a permissions test before locking the OID. It's true that
> permissions might be revoked after we test them and before the lock is
> acquired, but that doesn't seem terrible. The real point of all of
> this stuff is to keep users from locking objects which they never had
> any right to access, not to worry about what happens if permissions
> are concurrently revoked while we're getting the lock.

One thing is that neither pg_class_ownercheck nor pg_database_ownercheck
are fail-safe, and would issue an ERROR when the relation does not
exist, and this can happen when using multiple transactions for VACUUM
FULL or such, so we cannot simply swap the owner checks before trying to
lock the relation in vacuum_rel() and analyze_rel(). Or we invent new
flavors of those routine able to handle missing relations, then swap the
ACL checks to happen before the relations are locked.

For VACUUM/ANALYZE, I tend to think that it is incorrect to include from
the start in the list of relations to process all the ones a user is not
an owner of, so my approach seems quite natural, at least to me. Each
one of the two approaches has its good and bad sides.
--
Michael