• Tom Lane's avatar
    Fix inadequate locking during get_rel_oids(). · 19de0ab2
    Tom Lane authored
    get_rel_oids used to not take any relation locks at all, but that stopped
    being a good idea with commit 3c3bb993, which inserted a syscache lookup
    into the function.  A concurrent DROP TABLE could now produce "cache lookup
    failed", which we don't want to have happen in normal operation.  The best
    solution seems to be to transiently take a lock on the relation named by
    the RangeVar (which also makes the result of RangeVarGetRelid a lot less
    spongy).  But we shouldn't hold the lock beyond this function, because we
    don't want VACUUM to lock more than one table at a time.  (That would not
    be a big problem right now, but it will become one after the pending
    feature patch to allow multiple tables to be named in VACUUM.)
    
    In passing, adjust vacuum_rel and analyze_rel to document that we don't
    trust the passed RangeVar to be accurate, and allow the RangeVar to
    possibly be NULL --- which it is anyway for a whole-database VACUUM,
    though we accidentally didn't crash for that case.
    
    The passed RangeVar is in fact inaccurate when dealing with a child
    partition, as of v10, and it has been wrong for a whole long time in the
    case of vacuum_rel() recursing to a TOAST table.  None of these things
    present visible bugs up to now, because the passed RangeVar is in fact
    only consulted for autovacuum logging, and in that particular context it's
    always accurate because autovacuum doesn't let vacuum.c expand partitions
    nor recurse to toast tables.  Still, this seems like trouble waiting to
    happen, so let's nail the door at least partly shut.  (Further cleanup
    is planned, in HEAD only, as part of the pending feature patch.)
    
    Fix some sadly inaccurate/obsolete comments too.  Back-patch to v10.
    
    Michael Paquier and Tom Lane
    
    Discussion: https://postgr.es/m/25023.1506107590@sss.pgh.pa.us
    19de0ab2
analyze.c 84.6 KB