Don't to predicate lock for analyze scans, refactor scan option passing.
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
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/c3b23ae457ddc8b7bfacb3c0569278615a2df2cd
Modified Files
--------------
src/backend/access/heap/heapam.c | 120 ++++++++++++++++++-------------
src/backend/access/heap/heapam_handler.c | 6 +-
src/backend/access/table/tableam.c | 14 ++--
src/include/access/heapam.h | 7 +-
src/include/access/relscan.h | 13 ++--
src/include/access/tableam.h | 85 ++++++++++++++--------
src/test/regress/expected/vacuum.out | 8 +++
src/test/regress/sql/vacuum.sql | 9 +++
8 files changed, 160 insertions(+), 102 deletions(-)