From: Melanie Plageman Date: Mon, 12 Feb 2024 18:50:29 -0500 Subject: [PATCH v5 01/14] BitmapHeapScan begin scan after bitmap creation There is no reason for a BitmapHeapScan to begin the scan of the underlying table in ExecInitBitmapHeapScan(). Instead, do so after completing the index scan and building the bitmap. ExecBitmapHeapInitializeWorker() overwrote the snapshot in the scan descriptor with the correct one provided by the parallel leader. Since ExecBitmapHeapInitializeWorker() is now called before the scan descriptor has been created, save the worker's snapshot in the BitmapHeapScanState and pass it to table_beginscan_bm(). --- src/backend/access/table/tableam.c | 11 ------ src/backend/executor/nodeBitmapHeapscan.c | 47 ++++++++++++++++++----- src/include/access/tableam.h | 10 ++--- src/include/nodes/execnodes.h | 2 + 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 6ed8cca05a1..e78d793f69c 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -120,17 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key) NULL, flags); } -void -table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot) -{ - Assert(IsMVCCSnapshot(snapshot)); - - RegisterSnapshot(snapshot); - scan->rs_snapshot = snapshot; - scan->rs_flags |= SO_TEMP_SNAPSHOT; -} - - /* ---------------------------------------------------------------------------- * Parallel table scan related functions. * ---------------------------------------------------------------------------- diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index c1e81ebed63..44bf38be3c9 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -181,6 +181,34 @@ BitmapHeapNext(BitmapHeapScanState *node) } #endif /* USE_PREFETCH */ } + + /* + * If this is the first scan of the underlying table, create the table + * scan descriptor and begin the scan. + */ + if (!scan) + { + Snapshot snapshot = node->ss.ps.state->es_snapshot; + uint32 extra_flags = 0; + + /* + * Parallel workers must use the snapshot initialized by the + * parallel leader. + */ + if (node->worker_snapshot) + { + snapshot = node->worker_snapshot; + extra_flags |= SO_TEMP_SNAPSHOT; + } + + scan = node->ss.ss_currentScanDesc = table_beginscan_bm( + node->ss.ss_currentRelation, + snapshot, + 0, + NULL, + extra_flags); + } + node->initialized = true; } @@ -604,7 +632,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node) PlanState *outerPlan = outerPlanState(node); /* rescan to release any page pin */ - table_rescan(node->ss.ss_currentScanDesc, NULL); + if (node->ss.ss_currentScanDesc) + table_rescan(node->ss.ss_currentScanDesc, NULL); /* release bitmaps and buffers if any */ if (node->tbmiterator) @@ -681,7 +710,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) /* * close heap scan */ - table_endscan(scanDesc); + if (scanDesc) + table_endscan(scanDesc); + } /* ---------------------------------------------------------------- @@ -739,6 +770,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) */ scanstate->can_skip_fetch = (node->scan.plan.qual == NIL && node->scan.plan.targetlist == NIL); + scanstate->worker_snapshot = NULL; /* * Miscellaneous initialization @@ -787,11 +819,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->ss.ss_currentRelation = currentRelation; - scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation, - estate->es_snapshot, - 0, - NULL); - /* * all done. */ @@ -930,13 +957,13 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, ParallelWorkerContext *pwcxt) { ParallelBitmapHeapState *pstate; - Snapshot snapshot; Assert(node->ss.ps.state->es_query_dsa != NULL); pstate = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false); node->pstate = pstate; - snapshot = RestoreSnapshot(pstate->phs_snapshot_data); - table_scan_update_snapshot(node->ss.ss_currentScanDesc, snapshot); + node->worker_snapshot = RestoreSnapshot(pstate->phs_snapshot_data); + Assert(IsMVCCSnapshot(node->worker_snapshot)); + RegisterSnapshot(node->worker_snapshot); } diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 5f8474871d2..5375dd7150f 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -944,9 +944,10 @@ table_beginscan_strat(Relation rel, Snapshot snapshot, */ static inline TableScanDesc table_beginscan_bm(Relation rel, Snapshot snapshot, - int nkeys, struct ScanKeyData *key) + int nkeys, struct ScanKeyData *key, + uint32 extra_flags) { - uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE; + uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags; return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); } @@ -1038,11 +1039,6 @@ table_rescan_set_params(TableScanDesc scan, struct ScanKeyData *key, allow_pagemode); } -/* - * Update snapshot used by the scan. - */ -extern void table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot); - /* * Return next tuple from `scan`, store in slot. */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 444a5f0fd57..00c75fb10e2 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1726,6 +1726,7 @@ typedef struct ParallelBitmapHeapState * shared_tbmiterator shared iterator * shared_prefetch_iterator shared iterator for prefetching * pstate shared state for parallel bitmap scan + * worker_snapshot snapshot for parallel worker * ---------------- */ typedef struct BitmapHeapScanState @@ -1750,6 +1751,7 @@ typedef struct BitmapHeapScanState TBMSharedIterator *shared_tbmiterator; TBMSharedIterator *shared_prefetch_iterator; ParallelBitmapHeapState *pstate; + Snapshot worker_snapshot; } BitmapHeapScanState; /* ---------------- -- 2.37.2 --6kjpcnqyi64ibp5i Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"