From: Tomas Vondra Date: Tue, 7 Apr 2026 10:47:04 +0000 (+0200) Subject: Fix BitmapHeapScan non-parallel-aware EXPLAIN ANALYZE X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9c18b47e61071cf1395620f182b07a7ab9aac263;p=thirdparty%2Fpostgresql.git Fix BitmapHeapScan non-parallel-aware EXPLAIN ANALYZE Allocates shared bitmap table scan instrumentation for all parallel scans. Previously, the instrumentation was only allocated for parallel-aware scans, other bitmap heap scans in the parallel query had no shared instrumentation and EXPLAIN didn't report exact/lossy pages. This affected cases like scans on the outside of a parallel join or queries run with debug_parallel_query=regress. Fixed by allocating a separate DSM chunk for shared instrumentation and doing so regardless of parallel-awareness. The instrumentation is allocated in its own DSM chunk, separate from ParallelBitmapHeapState. Report an initial patch by me. The approach with a separate DSM was proposed and implemented by Melanie. Not backpatched. The issue affects Postgres 18 (since 5a1e6df3b84c), but having multiple DSM chunks is possible only since dd78e69cfc33. If we decide to fix this in backbranches too, it will need to be done in a less invasive way. Author: Melanie Plageman Reviewed-by: Tomas Vondra Reviewed-by: Lukas Fittl Discussion: https://postgr.es/m/flat/a177a6dd-240b-455a-8f25-aca0b1c08c6e%40vondra.me --- diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 73eaaf176ac..f151f21f9b3 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -3948,7 +3948,7 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es) } /* Display stats for each parallel worker */ - if (planstate->pstate != NULL) + if (planstate->sinstrument != NULL) { for (int n = 0; n < planstate->sinstrument->num_workers; n++) { diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index 726aca230a4..1a5ec0c305f 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -303,6 +303,9 @@ ExecParallelEstimate(PlanState *planstate, ExecParallelEstimateContext *e) if (planstate->plan->parallel_aware) ExecBitmapHeapEstimate((BitmapHeapScanState *) planstate, e->pcxt); + /* even when not parallel-aware, for EXPLAIN ANALYZE */ + ExecBitmapHeapInstrumentEstimate((BitmapHeapScanState *) planstate, + e->pcxt); break; case T_HashJoinState: if (planstate->plan->parallel_aware) @@ -542,6 +545,9 @@ ExecParallelInitializeDSM(PlanState *planstate, if (planstate->plan->parallel_aware) ExecBitmapHeapInitializeDSM((BitmapHeapScanState *) planstate, d->pcxt); + /* even when not parallel-aware, for EXPLAIN ANALYZE */ + ExecBitmapHeapInstrumentInitDSM((BitmapHeapScanState *) planstate, + d->pcxt); break; case T_HashJoinState: if (planstate->plan->parallel_aware) @@ -1427,6 +1433,9 @@ ExecParallelInitializeWorker(PlanState *planstate, ParallelWorkerContext *pwcxt) if (planstate->plan->parallel_aware) ExecBitmapHeapInitializeWorker((BitmapHeapScanState *) planstate, pwcxt); + /* even when not parallel-aware, for EXPLAIN ANALYZE */ + ExecBitmapHeapInstrumentInitWorker((BitmapHeapScanState *) planstate, + pwcxt); break; case T_HashJoinState: if (planstate->plan->parallel_aware) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 73831aed451..d65e2a87b42 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -495,18 +495,8 @@ void ExecBitmapHeapEstimate(BitmapHeapScanState *node, ParallelContext *pcxt) { - Size size; - - size = MAXALIGN(sizeof(ParallelBitmapHeapState)); - - /* account for instrumentation, if required */ - if (node->ss.ps.instrument && pcxt->nworkers > 0) - { - size = add_size(size, offsetof(SharedBitmapHeapInstrumentation, sinstrument)); - size = add_size(size, mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation))); - } - - shm_toc_estimate_chunk(&pcxt->estimator, size); + shm_toc_estimate_chunk(&pcxt->estimator, + MAXALIGN(sizeof(ParallelBitmapHeapState))); shm_toc_estimate_keys(&pcxt->estimator, 1); } @@ -521,27 +511,15 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node, ParallelContext *pcxt) { ParallelBitmapHeapState *pstate; - SharedBitmapHeapInstrumentation *sinstrument = NULL; dsa_area *dsa = node->ss.ps.state->es_query_dsa; - char *ptr; - Size size; /* If there's no DSA, there are no workers; initialize nothing. */ if (dsa == NULL) return; - size = MAXALIGN(sizeof(ParallelBitmapHeapState)); - if (node->ss.ps.instrument && pcxt->nworkers > 0) - { - size = add_size(size, offsetof(SharedBitmapHeapInstrumentation, sinstrument)); - size = add_size(size, mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation))); - } - - ptr = shm_toc_allocate(pcxt->toc, size); - pstate = (ParallelBitmapHeapState *) ptr; - ptr += MAXALIGN(sizeof(ParallelBitmapHeapState)); - if (node->ss.ps.instrument && pcxt->nworkers > 0) - sinstrument = (SharedBitmapHeapInstrumentation *) ptr; + pstate = (ParallelBitmapHeapState *) + shm_toc_allocate(pcxt->toc, + MAXALIGN(sizeof(ParallelBitmapHeapState))); pstate->tbmiterator = 0; @@ -551,18 +529,8 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node, ConditionVariableInit(&pstate->cv); - if (sinstrument) - { - sinstrument->num_workers = pcxt->nworkers; - - /* ensure any unfilled slots will contain zeroes */ - memset(sinstrument->sinstrument, 0, - pcxt->nworkers * sizeof(BitmapHeapScanInstrumentation)); - } - shm_toc_insert(pcxt->toc, node->ss.ps.plan->plan_node_id, pstate); node->pstate = pstate; - node->sinstrument = sinstrument; } /* ---------------------------------------------------------------- @@ -600,17 +568,72 @@ void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, ParallelWorkerContext *pwcxt) { - char *ptr; - Assert(node->ss.ps.state->es_query_dsa != NULL); - ptr = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false); + node->pstate = (ParallelBitmapHeapState *) + shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false); +} + +/* + * Compute the amount of space we'll need for the shared instrumentation and + * inform pcxt->estimator. + */ +void +ExecBitmapHeapInstrumentEstimate(BitmapHeapScanState *node, + ParallelContext *pcxt) +{ + Size size; + + if (!node->ss.ps.instrument || pcxt->nworkers == 0) + return; + + size = add_size(offsetof(SharedBitmapHeapInstrumentation, sinstrument), + mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation))); + shm_toc_estimate_chunk(&pcxt->estimator, size); + shm_toc_estimate_keys(&pcxt->estimator, 1); +} + +/* + * Set up parallel bitmap heap scan instrumentation. + */ +void +ExecBitmapHeapInstrumentInitDSM(BitmapHeapScanState *node, + ParallelContext *pcxt) +{ + Size size; + + if (!node->ss.ps.instrument || pcxt->nworkers == 0) + return; + + size = add_size(offsetof(SharedBitmapHeapInstrumentation, sinstrument), + mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation))); + node->sinstrument = + (SharedBitmapHeapInstrumentation *) shm_toc_allocate(pcxt->toc, size); + + /* Each per-worker area must start out as zeroes */ + memset(node->sinstrument, 0, size); + node->sinstrument->num_workers = pcxt->nworkers; + shm_toc_insert(pcxt->toc, + node->ss.ps.plan->plan_node_id + + PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET, + node->sinstrument); +} - node->pstate = (ParallelBitmapHeapState *) ptr; - ptr += MAXALIGN(sizeof(ParallelBitmapHeapState)); +/* + * Look up and save the location of the shared instrumentation. + */ +void +ExecBitmapHeapInstrumentInitWorker(BitmapHeapScanState *node, + ParallelWorkerContext *pwcxt) +{ + if (!node->ss.ps.instrument) + return; - if (node->ss.ps.instrument) - node->sinstrument = (SharedBitmapHeapInstrumentation *) ptr; + node->sinstrument = (SharedBitmapHeapInstrumentation *) + shm_toc_lookup(pwcxt->toc, + node->ss.ps.plan->plan_node_id + + PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET, + false); } /* ---------------------------------------------------------------- diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.h index 5d82f71abff..5c0b6592a1f 100644 --- a/src/include/executor/nodeBitmapHeapscan.h +++ b/src/include/executor/nodeBitmapHeapscan.h @@ -28,6 +28,12 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node, ParallelContext *pcxt); extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, ParallelWorkerContext *pwcxt); +extern void ExecBitmapHeapInstrumentEstimate(BitmapHeapScanState *node, + ParallelContext *pcxt); +extern void ExecBitmapHeapInstrumentInitDSM(BitmapHeapScanState *node, + ParallelContext *pcxt); +extern void ExecBitmapHeapInstrumentInitWorker(BitmapHeapScanState *node, + ParallelWorkerContext *pwcxt); extern void ExecBitmapHeapRetrieveInstrumentation(BitmapHeapScanState *node); #endif /* NODEBITMAPHEAPSCAN_H */