In [1] I whined about how the parallel heap scan machinery should have
noticed that the same ParallelTableScanDesc was being used to give out
block numbers for two different relations. Looking closer, there
are Asserts that mean to catch this type of error --- but they are
comparing relation OIDs, whereas what would have been needed to detect
the problem was to compare RelFileLocators.
It seems to me that a scan is fundamentally operating at the physical
relation level, and therefore these tests should check RelFileLocators
not OIDs. Hence I propose the attached. (For master only, of course;
this would be an ABI break in the back branches.) This passes
check-world and is able to catch the problem exposed in the other
thread.
Another possible view is that we should check both physical and
logical relation IDs, but that seems like overkill to me.
Thoughts?
regards, tom lane
[1] https://www.postgresql.org/message-id/2042942.1726781733%40sss.pgh.pa.us
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index dcd04b813d..1859be614c 100644
--- 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
index e57a0b7ea3..bd8715b679 100644
--- 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
index 521043304a..114a85dc47 100644
--- 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;
