From 7560f29cece7bfa4c75d3e9a0d8495543edbf5e5 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 9 Oct 2024 16:47:33 -0400 Subject: [PATCH v25 10/11] 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 | 6 ++ src/backend/access/heap/heapam_handler.c | 58 +++++++++----- src/backend/access/table/tableamapi.c | 2 - src/backend/executor/nodeBitmapHeapscan.c | 63 ++++++--------- src/backend/optimizer/util/plancat.c | 2 +- src/include/access/tableam.h | 94 ++++++----------------- 6 files changed, 91 insertions(+), 134 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index b8d8d0452aa..9b3878aff37 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -379,6 +379,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock) 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 @@ -1073,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. @@ -1186,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 dc4faa0643e..7bd8a17099d 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); @@ -2117,10 +2120,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 = (HeapScanDesc) bscan; @@ -2292,15 +2300,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 = (HeapScanDesc) bscan; @@ -2311,23 +2321,32 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan, ParallelBitmapHeapState *pstate = scan->st.bitmap.rs_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 >= 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 - */ - Assert(hscan->rs_cindex >= 0); - if (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 @@ -2953,7 +2972,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 e9b598256fb..912495bdaa1 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 f522ba4959a..86b27ec62b8 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -189,55 +189,36 @@ 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 b913f91ff03..29f5036e9b0 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 df7b02be9ca..52c6c4818cb 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -784,45 +784,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. + * 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. Tuples from lossy pages will - * always need to be rechecked, but some non-lossy pages' tuples may also - * require recheck. + * `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 future blocks indicated in the bitmap is left to the table - * AM. + * Prefetching additional data from 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. + * This is an optional callback. */ - bool (*scan_bitmap_next_block) (TableScanDesc scan, + bool (*scan_bitmap_next_tuple) (TableScanDesc scan, + TupleTableSlot *slot, 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. - */ - bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - TupleTableSlot *slot); - /* * Prepare to fetch tuples from the next block in a sample scan. Return * false if the sample scan is finished, true otherwise. `scan` was @@ -1956,51 +1934,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 @@ -2011,7 +1962,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); } /* -- 2.45.2