From 869023793654571cba6af0ccfd2fac9950ccc635 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 22 Mar 2024 15:43:10 -0400 Subject: [PATCH v15 13/13] Remove table_scan_bitmap_next_block() With several of the changes to the control flow of BitmapHeapNext() in recent commits, table_scan_bitmap_next_tuple() can be responsible for getting the next block. Do this and remove the table AM API function table_scan_bitmap_next_block(). Author: Melanie Plageman Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com --- src/backend/access/heap/heapam.c | 2 + src/backend/access/heap/heapam_handler.c | 48 ++++++++------- src/backend/access/table/tableamapi.c | 2 - src/backend/executor/nodeBitmapHeapscan.c | 61 +++++++++---------- src/backend/optimizer/util/plancat.c | 2 +- src/include/access/tableam.h | 74 +++++------------------ 6 files changed, 73 insertions(+), 116 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index a21ec92d71..9c98109a16 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -324,6 +324,8 @@ 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; /* page-at-a-time fields are always invalid when not rs_inited */ diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index c513cbd2da..d7f3bb960f 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2183,12 +2183,6 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths, HEAP_USABLE_BYTES_PER_PAGE); } - -/* ------------------------------------------------------------------------ - * Executor related callbacks for the heap AM - * ------------------------------------------------------------------------ - */ - /* * BitmapAdjustPrefetchIterator - Adjust the prefetch iterator * @@ -2222,8 +2216,8 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanDesc scan) /* * Adjusting the prefetch iterator before invoking - * table_scan_bitmap_next_block() keeps prefetch distance higher across - * the parallel workers. + * heapam_bitmap_next_block() keeps prefetch distance higher across the + * parallel workers. */ if (scan->prefetch_maximum > 0) { @@ -2583,9 +2577,15 @@ BitmapPrefetch(BitmapHeapScanDesc scan) #endif /* USE_PREFETCH */ } +/* ------------------------------------------------------------------------ + * Executor related callbacks for the heap AM + * ------------------------------------------------------------------------ + */ + static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, - TupleTableSlot *slot) + TupleTableSlot *slot, bool *recheck, + long *lossy_pages, long *exact_pages) { BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan; HeapScanDesc hscan = &bscan->heap_common; @@ -2593,21 +2593,28 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan, Page page; ItemId lp; - if (bscan->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->empty_tuples_pending--; - return true; - } + if (bscan->empty_tuples_pending > 0) + { + /* + * If we don't have to fetch the tuple, just return nulls. + */ + ExecStoreAllNullTuple(slot); + bscan->empty_tuples_pending--; + 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; + if (!heapam_scan_bitmap_next_block(scan, recheck, + lossy_pages, exact_pages)) + return false; + } #ifdef USE_PREFETCH @@ -3008,7 +3015,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 55b8caeadf..0b87530a9c 100644 --- a/src/backend/access/table/tableamapi.c +++ b/src/backend/access/table/tableamapi.c @@ -90,8 +90,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 aaeece6697..48086aae1d 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -206,58 +206,53 @@ BitmapHeapInitialize(BitmapHeapScanState *node) static TupleTableSlot * BitmapHeapNext(BitmapHeapScanState *node) { - ExprContext *econtext = node->ss.ps.ps_ExprContext; - TupleTableSlot *slot = node->ss.ss_ScanTupleSlot; - TableScanDesc scan = node->ss.ss_currentScanDesc; + ExprContext *econtext; + TupleTableSlot *slot; + TableScanDesc scan; /* * If we haven't yet performed the underlying index scan, do it, and begin * the iteration over the bitmap. This happens on rescan as well. */ if (!node->initialized) - { BitmapHeapInitialize(node); - /* We may have a new scan descriptor */ - scan = node->ss.ss_currentScanDesc; - goto new_page; - } + /* + * BitmapHeapInitialize() may make the scan descriptor so don't get it + * from the node until after calling it. + */ + econtext = node->ss.ps.ps_ExprContext; + slot = node->ss.ss_ScanTupleSlot; + scan = node->ss.ss_currentScanDesc; - for (;;) + while (table_scan_bitmap_next_tuple(scan, slot, &node->recheck, + &node->lossy_pages, &node->exact_pages)) { - while (table_scan_bitmap_next_tuple(scan, slot)) - { - CHECK_FOR_INTERRUPTS(); + 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: - - if (!table_scan_bitmap_next_block(scan, &node->recheck, - &node->lossy_pages, &node->exact_pages)) - break; + /* OK to return this tuple */ + return slot; } + /* * if we get here it means we are at the end of the scan.. */ diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 6bb53e4346..cf56cc572f 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -313,7 +313,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 138bf5f6ed..e4d0c4b0ed 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -790,34 +790,20 @@ typedef struct TableAmRoutine * ------------------------------------------------------------------------ */ - /* - * Prepare to fetch / check / return tuples 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 is incremented if the bitmap is lossy for the selected - * block; otherwise, exact_pages is incremented. - * - * 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, - long *lossy_pages, long *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. + * recheck is set if recheck is required. + * + * The table AM is responsible for reading in blocks and counting (for + * EXPLAIN) which of those blocks were represented lossily in the bitmap + * using the lossy_pages and exact_pages counters. */ bool (*scan_bitmap_next_tuple) (TableScanDesc scan, - TupleTableSlot *slot); + TupleTableSlot *slot, + bool *recheck, + long *lossy_pages, long *exact_pages); /* * Prepare to fetch tuples from the next block in a sample scan. Return @@ -1980,45 +1966,13 @@ 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 is - * incremented is the block's representation in the bitmap is lossy; otherwise, - * exact_pages is incremented. - * - * Note, this is an optionally implemented function, therefore should only be - * used after verifying the presence (at plan time or such). - */ -static inline bool -table_scan_bitmap_next_block(TableScanDesc scan, - bool *recheck, - long *lossy_pages, - long *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. + * Fetch the next tuple of a bitmap table scan into `slot` and return true if a + * visible tuple was found, false otherwise. */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - TupleTableSlot *slot) + TupleTableSlot *slot, bool *recheck, + long *lossy_pages, long *exact_pages) { /* * We don't expect direct calls to table_scan_bitmap_next_tuple with valid @@ -2029,7 +1983,9 @@ 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.40.1