Improve Asserts checking relation matching in parallel scans.

tglsfdc · tglsfdc · commit 54562c9cfa22 · 2024-09-20T16:37:55.000-04:00
table_beginscan_parallel and index_beginscan_parallel contain Asserts checking that the relation a worker will use in a parallel scan is the same one the leader intended. However, they were checking for relation OID match, which was not strong enough to detect the mismatch problem fixed in 126ec0b. What would be strong enough is to compare relfilenodes instead. Arguably, that's a saner definition anyway, since a scan surely operates on a physical relation not a logical one. Hence, store and compare RelFileLocators not relation OIDs. Also ensure that index_beginscan_parallel checks the index identity not just the table identity. Discussion: https://postgr.es/m/2127254.1726789524@sss.pgh.pa.us
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
@@ -500,8 +500,8 @@ index_parallelscan_initialize(Relation heapRelation, Relation indexRelation,
 					  EstimateSnapshotSpace(snapshot));
 	offset = MAXALIGN(offset);
 
-	target->ps_relid = RelationGetRelid(heapRelation);
-	target->ps_indexid = RelationGetRelid(indexRelation);
+	target->ps_locator = heapRelation->rd_locator;
+	target->ps_indexlocator = indexRelation->rd_locator;
 	target->ps_offset = offset;
 	SerializeSnapshot(snapshot, target->ps_snapshot_data);
 
@@ -544,7 +544,9 @@ index_beginscan_parallel(Relation heaprel, Relation indexrel, int nkeys,
 	Snapshot	snapshot;
 	IndexScanDesc scan;
 
-	Assert(RelationGetRelid(heaprel) == pscan->ps_relid);
+	Assert(RelFileLocatorEquals(heaprel->rd_locator, pscan->ps_locator));
+	Assert(RelFileLocatorEquals(indexrel->rd_locator, pscan->ps_indexlocator));
+
 	snapshot = RestoreSnapshot(pscan->ps_snapshot_data);
 	RegisterSnapshot(snapshot);
 	scan = index_beginscan_internal(indexrel, nkeys, norderbys, snapshot,
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
@@ -168,7 +168,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan)
 	uint32		flags = SO_TYPE_SEQSCAN |
 		SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
 
-	Assert(RelationGetRelid(relation) == pscan->phs_relid);
+	Assert(RelFileLocatorEquals(relation->rd_locator, pscan->phs_locator));
 
 	if (!pscan->phs_snapshot_any)
 	{
@@ -389,7 +389,7 @@ table_block_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan)
 {
 	ParallelBlockTableScanDesc bpscan = (ParallelBlockTableScanDesc) pscan;
 
-	bpscan->base.phs_relid = RelationGetRelid(rel);
+	bpscan->base.phs_locator = rel->rd_locator;
 	bpscan->phs_nblocks = RelationGetNumberOfBlocks(rel);
 	/* compare phs_syncscan initialization to similar logic in initscan */
 	bpscan->base.phs_syncscan = synchronize_seqscans &&
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
@@ -18,6 +18,7 @@
 #include "access/itup.h"
 #include "port/atomics.h"
 #include "storage/buf.h"
+#include "storage/relfilelocator.h"
 #include "storage/spin.h"
 #include "utils/relcache.h"
 
@@ -62,7 +63,7 @@ typedef struct TableScanDescData *TableScanDesc;
  */
 typedef struct ParallelTableScanDescData
 {
-	Oid			phs_relid;		/* OID of relation to scan */
+	RelFileLocator phs_locator; /* physical relation to scan */
 	bool		phs_syncscan;	/* report location to syncscan logic? */
 	bool		phs_snapshot_any;	/* SnapshotAny, not phs_snapshot_data? */
 	Size		phs_snapshot_off;	/* data for snapshot */
@@ -169,8 +170,8 @@ typedef struct IndexScanDescData
 /* Generic structure for parallel scans */
 typedef struct ParallelIndexScanDescData
 {
-	Oid			ps_relid;
-	Oid			ps_indexid;
+	RelFileLocator ps_locator;	/* physical table relation to scan */
+	RelFileLocator ps_indexlocator; /* physical index relation to scan */
 	Size		ps_offset;		/* Offset in bytes of am specific structure */
 	char		ps_snapshot_data[FLEXIBLE_ARRAY_MEMBER];
 }			ParallelIndexScanDescData;

Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan)`
`168`	`168`	`uint32 flags = SO_TYPE_SEQSCAN \|`
`169`	`169`	`SO_ALLOW_STRAT \| SO_ALLOW_SYNC \| SO_ALLOW_PAGEMODE;`
`170`	`170`
`171`		`- Assert(RelationGetRelid(relation) == pscan->phs_relid);`
	`171`	`+ Assert(RelFileLocatorEquals(relation->rd_locator, pscan->phs_locator));`
`172`	`172`
`173`	`173`	`if (!pscan->phs_snapshot_any)`
`174`	`174`	`{`
`@@ -389,7 +389,7 @@ table_block_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan)`
`389`	`389`	`{`
`390`	`390`	`ParallelBlockTableScanDesc bpscan = (ParallelBlockTableScanDesc) pscan;`
`391`	`391`
`392`		`- bpscan->base.phs_relid = RelationGetRelid(rel);`
	`392`	`+ bpscan->base.phs_locator = rel->rd_locator;`
`393`	`393`	`bpscan->phs_nblocks = RelationGetNumberOfBlocks(rel);`
`394`	`394`	`/* compare phs_syncscan initialization to similar logic in initscan */`
`395`	`395`	`bpscan->base.phs_syncscan = synchronize_seqscans &&`