• Tom Lane's avatar
    Make queries' locking of indexes more consistent. · 9c703c16
    Tom Lane authored
    The assertions added by commit b04aeb0a exposed that there are some
    code paths wherein the executor will try to open an index without
    holding any lock on it.  We do have some lock on the index's table,
    so it seems likely that there's no fatal problem with this (for
    instance, the index couldn't get dropped from under us).  Still,
    it's bad practice and we should fix it.
    
    To do so, remove the optimizations in ExecInitIndexScan and friends
    that tried to avoid taking a lock on an index belonging to a target
    relation, and just take the lock always.  In non-bug cases, this
    will result in no additional shared-memory access, since we'll find
    in the local lock table that we already have a lock of the desired
    type; hence, no significant performance degradation should occur.
    
    Also, adjust the planner and executor so that the type of lock taken
    on an index is always identical to the type of lock taken for its table,
    by relying on the recently added RangeTblEntry.rellockmode field.
    This avoids some corner cases where that might not have been true
    before (possibly resulting in extra locking overhead), and prevents
    future maintenance issues from having multiple bits of logic that
    all needed to be in sync.  In addition, this change removes all core
    calls to ExecRelationIsTargetRelation, which avoids a possible O(N^2)
    startup penalty for queries with large numbers of target relations.
    (We'd probably remove that function altogether, were it not that we
    advertise it as something that FDWs might want to use.)
    
    Also adjust some places in selfuncs.c to not take any lock on indexes
    they are transiently opening, since we can assume that plancat.c
    did that already.
    
    In passing, change gin_clean_pending_list() to take RowExclusiveLock
    not AccessShareLock on its target index.  Although it's not clear that
    that's actually a bug, it seemed very strange for a function that's
    explicitly going to modify the index to use only AccessShareLock.
    
    David Rowley, reviewed by Julien Rouhaud and Amit Langote,
    a bit of further tweaking by me
    
    Discussion: https://postgr.es/m/19465.1541636036@sss.pgh.pa.us
    9c703c16
nodeIndexonlyscan.c 23 KB