1
0
mirror of https://github.com/postgres/postgres.git synced 2025-08-05 07:41:25 +03:00

Improve Asserts checking relation matching in parallel scans.

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 126ec0bc7.
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
This commit is contained in:
Tom Lane
2024-09-20 16:37:55 -04:00
parent afb03e2ebf
commit 54562c9cfa
3 changed files with 11 additions and 8 deletions

View File

@@ -500,8 +500,8 @@ index_parallelscan_initialize(Relation heapRelation, Relation indexRelation,
EstimateSnapshotSpace(snapshot)); EstimateSnapshotSpace(snapshot));
offset = MAXALIGN(offset); offset = MAXALIGN(offset);
target->ps_relid = RelationGetRelid(heapRelation); target->ps_locator = heapRelation->rd_locator;
target->ps_indexid = RelationGetRelid(indexRelation); target->ps_indexlocator = indexRelation->rd_locator;
target->ps_offset = offset; target->ps_offset = offset;
SerializeSnapshot(snapshot, target->ps_snapshot_data); SerializeSnapshot(snapshot, target->ps_snapshot_data);
@@ -544,7 +544,9 @@ index_beginscan_parallel(Relation heaprel, Relation indexrel, int nkeys,
Snapshot snapshot; Snapshot snapshot;
IndexScanDesc scan; 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); snapshot = RestoreSnapshot(pscan->ps_snapshot_data);
RegisterSnapshot(snapshot); RegisterSnapshot(snapshot);
scan = index_beginscan_internal(indexrel, nkeys, norderbys, snapshot, scan = index_beginscan_internal(indexrel, nkeys, norderbys, snapshot,

View File

@@ -168,7 +168,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan)
uint32 flags = SO_TYPE_SEQSCAN | uint32 flags = SO_TYPE_SEQSCAN |
SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE; 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) if (!pscan->phs_snapshot_any)
{ {
@@ -389,7 +389,7 @@ table_block_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan)
{ {
ParallelBlockTableScanDesc bpscan = (ParallelBlockTableScanDesc) pscan; ParallelBlockTableScanDesc bpscan = (ParallelBlockTableScanDesc) pscan;
bpscan->base.phs_relid = RelationGetRelid(rel); bpscan->base.phs_locator = rel->rd_locator;
bpscan->phs_nblocks = RelationGetNumberOfBlocks(rel); bpscan->phs_nblocks = RelationGetNumberOfBlocks(rel);
/* compare phs_syncscan initialization to similar logic in initscan */ /* compare phs_syncscan initialization to similar logic in initscan */
bpscan->base.phs_syncscan = synchronize_seqscans && bpscan->base.phs_syncscan = synchronize_seqscans &&

View File

@@ -18,6 +18,7 @@
#include "access/itup.h" #include "access/itup.h"
#include "port/atomics.h" #include "port/atomics.h"
#include "storage/buf.h" #include "storage/buf.h"
#include "storage/relfilelocator.h"
#include "storage/spin.h" #include "storage/spin.h"
#include "utils/relcache.h" #include "utils/relcache.h"
@@ -62,7 +63,7 @@ typedef struct TableScanDescData *TableScanDesc;
*/ */
typedef struct ParallelTableScanDescData 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_syncscan; /* report location to syncscan logic? */
bool phs_snapshot_any; /* SnapshotAny, not phs_snapshot_data? */ bool phs_snapshot_any; /* SnapshotAny, not phs_snapshot_data? */
Size phs_snapshot_off; /* data for snapshot */ Size phs_snapshot_off; /* data for snapshot */
@@ -169,8 +170,8 @@ typedef struct IndexScanDescData
/* Generic structure for parallel scans */ /* Generic structure for parallel scans */
typedef struct ParallelIndexScanDescData typedef struct ParallelIndexScanDescData
{ {
Oid ps_relid; RelFileLocator ps_locator; /* physical table relation to scan */
Oid ps_indexid; RelFileLocator ps_indexlocator; /* physical index relation to scan */
Size ps_offset; /* Offset in bytes of am specific structure */ Size ps_offset; /* Offset in bytes of am specific structure */
char ps_snapshot_data[FLEXIBLE_ARRAY_MEMBER]; char ps_snapshot_data[FLEXIBLE_ARRAY_MEMBER];
} ParallelIndexScanDescData; } ParallelIndexScanDescData;