• Andres Freund's avatar
    Don't to predicate lock for analyze scans, refactor scan option passing. · c3b23ae4
    Andres Freund authored
    Before this commit, when ANALYZE was run on a table and serializable
    was used (either by virtue of an explicit BEGIN TRANSACTION ISOLATION
    LEVEL SERIALIZABLE, or default_transaction_isolation being set to
    serializable) a null pointer dereference lead to a crash.
    
    The analyze scan doesn't need a snapshot (nor predicate locking), but
    before this commit a scan only contained information about being a
    bitmap or sample scan.
    
    Refactor the option passing to the scan_begin callback to use a
    bitmask instead. Alternatively we could have added a new boolean
    parameter, but that seems harder to read. Even before this issue
    various people (Heikki, Tom, Robert) suggested doing so.
    
    These changes don't change the scan APIs outside of tableam. The flags
    argument could be exposed, it's not necessary to fix this
    problem. Also the wrapper table_beginscan* functions encapsulate most
    of that complexity.
    
    After these changes fixing the bug is trivial, just don't acquire
    predicate lock for analyze style scans. That was already done for
    bitmap heap scans.  Add an assert that a snapshot is passed when
    acquiring the predicate lock, so this kind of bug doesn't require
    running with serializable.
    
    Also add a comment about sample scans currently requiring predicate
    locking the entire relation, that previously wasn't remarked upon.
    
    Reported-By: Joe Wildish
    Author: Andres Freund
    Discussion:
        https://postgr.es/m/4EA80A20-E9BF-49F1-9F01-5B66CAB21453@elusive.cx
        https://postgr.es/m/20190411164947.nkii4gaeilt4bui7@alap3.anarazel.de
        https://postgr.es/m/20190518203102.g7peu2fianukjuxm@alap3.anarazel.de
    c3b23ae4
vacuum.out 12.3 KB