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/[email protected]
    https://postgr.es/m/[email protected]
    https://postgr.es/m/[email protected]

1 files changed, 8 insertions, 6 deletions

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 73025b8ead0..c3455bc48ba 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -93,12 +93,13 @@ table_slot_create(Relation relation, List **reglist)
 TableScanDesc
 table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 {
+	uint32		flags = SO_TYPE_SEQSCAN |
+		SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE | SO_TEMP_SNAPSHOT;
 	Oid			relid = RelationGetRelid(relation);
 	Snapshot	snapshot = RegisterSnapshot(GetCatalogSnapshot(relid));
 
 	return relation->rd_tableam->scan_begin(relation, snapshot, nkeys, key,
-											NULL, true, true, true, false,
-											false, true);
+											NULL, flags);
 }
 
 void
@@ -108,7 +109,7 @@ table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot)
 
 	RegisterSnapshot(snapshot);
 	scan->rs_snapshot = snapshot;
-	scan->rs_temp_snap = true;
+	scan->rs_flags |= SO_TEMP_SNAPSHOT;
 }
 
 
@@ -156,6 +157,8 @@ TableScanDesc
 table_beginscan_parallel(Relation relation, ParallelTableScanDesc parallel_scan)
 {
 	Snapshot	snapshot;
+	uint32		flags = SO_TYPE_SEQSCAN |
+		SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
 
 	Assert(RelationGetRelid(relation) == parallel_scan->phs_relid);
 
@@ -165,6 +168,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc parallel_scan)
 		snapshot = RestoreSnapshot((char *) parallel_scan +
 								   parallel_scan->phs_snapshot_off);
 		RegisterSnapshot(snapshot);
+		flags |= SO_TEMP_SNAPSHOT;
 	}
 	else
 	{
@@ -173,9 +177,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc parallel_scan)
 	}
 
 	return relation->rd_tableam->scan_begin(relation, snapshot, 0, NULL,
-											parallel_scan, true, true, true,
-											false, false,
-											!parallel_scan->phs_snapshot_any);
+											parallel_scan, flags);
 }