From 60f37859c7108e912980a3884d0c7b767f0a959b Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 9 Oct 2024 16:47:33 -0400 Subject: [PATCH v24 11/12] Remove table AM callback scan_bitmap_next_block Remove table AM callback scan_bitmap_next_block(). This makes implementing bitmap table scans easier for non block-based table AMs and makes BitmapHeapNext() control flow similar to SeqNext(). Now bitmap table scan code only calls table_scan_bitmap_next_tuple(). The heap AM implementation of scan_bitmap_next_block() is now a local helper in heapam_handler.c. --- src/backend/access/heap/heapam.c | 7 ++ src/backend/access/heap/heapam_handler.c | 57 +++++++++----- src/backend/access/table/tableamapi.c | 2 - src/backend/executor/nodeBitmapHeapscan.c | 57 +++++--------- src/backend/optimizer/util/plancat.c | 2 +- src/include/access/tableam.h | 96 ++++++----------------- src/include/nodes/execnodes.h | 4 - 7 files changed, 92 insertions(+), 133 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 4ea8933c44..c673457889 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -378,6 +378,9 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock) ItemPointerSetInvalid(&scan->rs_ctup.t_self); scan->rs_cbuf = InvalidBuffer; scan->rs_cblock = InvalidBlockNumber; + scan->rs_cindex = 0; + scan->rs_ntuples = 0; + /* * Initialize to ForwardScanDirection because it is most common and @@ -1072,6 +1075,7 @@ heap_beginscan(Relation relation, Snapshot snapshot, scan->rs_base.rs_flags = flags; scan->rs_base.rs_parallel = parallel_scan; scan->rs_strategy = NULL; /* set in initscan */ + scan->rs_cbuf = InvalidBuffer; /* * Disable page-at-a-time mode if it's not a MVCC-safe snapshot. @@ -1185,7 +1189,10 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params, * unpin scan buffers */ if (BufferIsValid(scan->rs_cbuf)) + { ReleaseBuffer(scan->rs_cbuf); + scan->rs_cbuf = InvalidBuffer; + } if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN) { diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 78193b1248..6fc779e1d0 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -57,6 +57,9 @@ static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer, static void BitmapAdjustPrefetchIterator(BitmapHeapScanDesc *bscan); static void BitmapAdjustPrefetchTarget(BitmapHeapScanDesc *bscan); static void BitmapPrefetch(BitmapHeapScanDesc *bscan); +static bool BitmapHeapScanNextBlock(TableScanDesc scan, + bool *recheck, + uint64 *lossy_pages, uint64 *exact_pages); static BlockNumber heapam_scan_get_blocks_done(HeapScanDesc hscan); @@ -2118,10 +2121,15 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths, * ------------------------------------------------------------------------ */ +/* + * Helper function get the next block of a bitmap heap scan. Returns true when + * it got the next block and saved it in the scan descriptor and false when the + * bitmap and or relation are exhausted. + */ static bool -heapam_scan_bitmap_next_block(TableScanDesc scan, - bool *recheck, - uint64 *lossy_pages, uint64 *exact_pages) +BitmapHeapScanNextBlock(TableScanDesc scan, + bool *recheck, + uint64 *lossy_pages, uint64 *exact_pages) { BitmapHeapScanDesc *bscan = (BitmapHeapScanDesc *) scan; HeapScanDesc hscan = &bscan->rs_heap_base; @@ -2288,15 +2296,17 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, * Return true to indicate that a valid block was found and the bitmap is * not exhausted. If there are no visible tuples on this page, * hscan->rs_ntuples will be 0 and heapam_scan_bitmap_next_tuple() will - * return false returning control to this function to advance to the next - * block in the bitmap. + * invoke this function again. */ return true; } static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TupleTableSlot *slot) + TupleTableSlot *slot, + bool *recheck, + uint64 *lossy_pages, + uint64 *exact_pages) { BitmapHeapScanDesc *bscan = (BitmapHeapScanDesc *) scan; HeapScanDesc hscan = &bscan->rs_heap_base; @@ -2307,22 +2317,32 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan, ParallelBitmapHeapState *pstate = scan->st.bts.pstate; #endif /* USE_PREFETCH */ - if (bscan->rs_empty_tuples_pending > 0) + /* + * Out of range? If so, nothing more to look at on this page + */ + while (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples) { /* - * If we don't have to fetch the tuple, just return nulls. + * Emit empty tuples before advancing to the next block */ - ExecStoreAllNullTuple(slot); - bscan->rs_empty_tuples_pending--; - BitmapPrefetch(bscan); - return true; - } + if (bscan->rs_empty_tuples_pending > 0) + { + /* + * If we don't have to fetch the tuple, just return nulls. + */ + ExecStoreAllNullTuple(slot); + bscan->rs_empty_tuples_pending--; + BitmapPrefetch(bscan); + return true; + } - /* - * Out of range? If so, nothing more to look at on this page - */ - if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples) - return false; + /* + * Returns false if the bitmap is exhausted and there are no further + * blocks we need to scan. + */ + if (!BitmapHeapScanNextBlock(scan, recheck, lossy_pages, exact_pages)) + return false; + } #ifdef USE_PREFETCH @@ -2941,7 +2961,6 @@ static const TableAmRoutine heapam_methods = { .relation_estimate_size = heapam_estimate_rel_size, - .scan_bitmap_next_block = heapam_scan_bitmap_next_block, .scan_bitmap_next_tuple = heapam_scan_bitmap_next_tuple, .scan_sample_next_block = heapam_scan_sample_next_block, .scan_sample_next_tuple = heapam_scan_sample_next_tuple diff --git a/src/backend/access/table/tableamapi.c b/src/backend/access/table/tableamapi.c index e9b598256f..912495bdaa 100644 --- a/src/backend/access/table/tableamapi.c +++ b/src/backend/access/table/tableamapi.c @@ -92,8 +92,6 @@ GetTableAmRoutine(Oid amhandler) Assert(routine->relation_estimate_size != NULL); /* optional, but one callback implies presence of the other */ - Assert((routine->scan_bitmap_next_block == NULL) == - (routine->scan_bitmap_next_tuple == NULL)); Assert(routine->scan_sample_next_block != NULL); Assert(routine->scan_sample_next_tuple != NULL); diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index ca22cd4196..25cfc1647a 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -181,51 +181,34 @@ BitmapHeapNext(BitmapHeapScanState *node) * the iteration over the bitmap. */ if (!node->initialized) - { - /* BitmapTableScanSetup sets node->ss.ss_currentScanDesc */ BitmapTableScanSetup(node); - goto new_page; - } - for (;;) + while (table_scan_bitmap_next_tuple(node->ss.ss_currentScanDesc, slot, &node->recheck, + &node->stats.lossy_pages, &node->stats.exact_pages)) { - while (table_scan_bitmap_next_tuple(node->ss.ss_currentScanDesc, slot)) - { - /* - * Continuing in previously obtained page. - */ - - CHECK_FOR_INTERRUPTS(); + /* + * Continuing in previously obtained page. + */ + CHECK_FOR_INTERRUPTS(); - /* - * If we are using lossy info, we have to recheck the qual - * conditions at every tuple. - */ - if (node->recheck) + /* + * If we are using lossy info, we have to recheck the qual conditions + * at every tuple. + */ + if (node->recheck) + { + econtext->ecxt_scantuple = slot; + if (!ExecQualAndReset(node->bitmapqualorig, econtext)) { - econtext->ecxt_scantuple = slot; - if (!ExecQualAndReset(node->bitmapqualorig, econtext)) - { - /* Fails recheck, so drop it and loop back for another */ - InstrCountFiltered2(node, 1); - ExecClearTuple(slot); - continue; - } + /* Fails recheck, so drop it and loop back for another */ + InstrCountFiltered2(node, 1); + ExecClearTuple(slot); + continue; } - - /* OK to return this tuple */ - return slot; } -new_page: - - /* - * Returns false if the bitmap is exhausted and there are no further - * blocks we need to scan. - */ - if (!table_scan_bitmap_next_block(node->ss.ss_currentScanDesc, &node->recheck, - &node->stats.lossy_pages, &node->stats.exact_pages)) - break; + /* OK to return this tuple */ + return slot; } /* diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index b913f91ff0..29f5036e9b 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -325,7 +325,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, info->amcanparallel = amroutine->amcanparallel; info->amhasgettuple = (amroutine->amgettuple != NULL); info->amhasgetbitmap = amroutine->amgetbitmap != NULL && - relation->rd_tableam->scan_bitmap_next_block != NULL; + relation->rd_tableam->scan_bitmap_next_tuple != NULL; info->amcanmarkpos = (amroutine->ammarkpos != NULL && amroutine->amrestrpos != NULL); info->amcostestimate = amroutine->amcostestimate; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 0b4cacb631..3339043a8a 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -783,44 +783,23 @@ typedef struct TableAmRoutine * ------------------------------------------------------------------------ */ - /* - * Prepare to fetch / check / return tuples from the next block yielded by - * the iterator as part of a bitmap table scan. `scan` was started via - * table_beginscan_bm(). Return false if the bitmap is exhausted and true - * otherwise. - * - * This will typically read and pin the target block, and do the necessary - * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might - * make sense to perform tuple visibility checks at this time). - * - * `lossy_pages` and `exact_pages` are EXPLAIN counters that can be - * incremented by the table AM to indicate whether or not the block's - * representation in the bitmap is lossy. - * - * `recheck` is set by the table AM to indicate whether or not the tuples - * from this block should be rechecked. Tuples from lossy pages will - * always need to be rechecked, but some non-lossy pages' tuples may also - * require recheck. - * - * Prefetching future blocks indicated in the bitmap is left to the table - * AM. - * - * Optional callback, but either both scan_bitmap_next_block and - * scan_bitmap_next_tuple need to exist, or neither. - */ - bool (*scan_bitmap_next_block) (TableScanDesc scan, - bool *recheck, - uint64 *lossy_pages, uint64 *exact_pages); - /* * Fetch the next tuple of a bitmap table scan into `slot` and return true * if a visible tuple was found, false otherwise. * - * Optional callback, but either both scan_bitmap_next_block and - * scan_bitmap_next_tuple need to exist, or neither. + * `lossy_pages` is incremented if the bitmap is lossy for the selected + * page; otherwise, `exact_pages` is incremented. This is tracked for + * display in EXPLAIN ANALYZE output. + * + * Prefetching additional data from the bitmap is left to the table AM. + * + * This is an optional callback. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - TupleTableSlot *slot); + TupleTableSlot *slot, + bool *recheck, + uint64 *lossy_pages, + uint64 *exact_pages); /* * Prepare to fetch tuples from the next block in a sample scan. Return @@ -1949,50 +1928,24 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths, */ /* - * Prepare to fetch / check / return tuples as part of a bitmap table scan. - * `scan` needs to have been started via table_beginscan_bm(). Returns false if - * there are no more blocks in the bitmap, true otherwise. - * - * `lossy_pages` and `exact_pages` are EXPLAIN counters that can be incremented - * by the table AM to indicate whether or not the block's representation in the - * bitmap is lossy. + * Fetch / check / return tuples as part of a bitmap table scan. `scan` needs + * to have been started via table_beginscan_bm(). Fetch the next tuple of a + * bitmap table scan into `slot` and return true if a visible tuple was found, + * false otherwise. * - * `recheck` is set by the table AM to indicate whether or not the tuples - * from this block should be rechecked. + * `recheck` is set by the table AM to indicate whether or not the tuple in + * `slot` should be rechecked. Tuples from lossy pages will always need to be + * rechecked, but some non-lossy pages' tuples may also require recheck. * - * Note, this is an optionally implemented function, therefore should only be - * used after verifying the presence (at plan time or such). + * `lossy_pages` is incremented if the block's representation in the bitmap is + * lossy; otherwise, `exact_pages` is incremented. */ static inline bool -table_scan_bitmap_next_block(TableScanDesc scan, +table_scan_bitmap_next_tuple(TableScanDesc scan, + TupleTableSlot *slot, bool *recheck, uint64 *lossy_pages, uint64 *exact_pages) -{ - /* - * We don't expect direct calls to table_scan_bitmap_next_block with valid - * CheckXidAlive for catalog or regular tables. See detailed comments in - * xact.c where these variables are declared. - */ - if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan)) - elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding"); - - return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan, - recheck, - lossy_pages, exact_pages); -} - -/* - * Fetch the next tuple of a bitmap table scan into `slot` and return true if - * a visible tuple was found, false otherwise. - * table_scan_bitmap_next_block() needs to previously have selected a - * block (i.e. returned true), and no previous - * table_scan_bitmap_next_tuple() for the same block may have - * returned false. - */ -static inline bool -table_scan_bitmap_next_tuple(TableScanDesc scan, - TupleTableSlot *slot) { /* * We don't expect direct calls to table_scan_bitmap_next_tuple with valid @@ -2003,7 +1956,10 @@ table_scan_bitmap_next_tuple(TableScanDesc scan, elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding"); return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan, - slot); + slot, + recheck, + lossy_pages, + exact_pages); } /* diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index d5bca3ac73..53fcb9d33a 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1846,10 +1846,6 @@ typedef struct BitmapHeapScanState ExprState *bitmapqualorig; TIDBitmap *tbm; BitmapHeapScanInstrumentation stats; - TBMIterator prefetch_iterator; - int prefetch_pages; - int prefetch_target; - int prefetch_maximum; bool initialized; ParallelBitmapHeapState *pstate; SharedBitmapHeapInstrumentation *sinstrument; -- 2.45.2