From feec27e011ef4f1f15e65afca2564192b85b17c1 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 22 Mar 2024 15:43:10 -0400 Subject: [PATCH v10 15/17] 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(). Heap AM's implementation of table_scan_bitmap_next_tuple() now calls the original heapam_scan_bitmap_next_block() function, but it is no longer an implementation of a table AM callback but instead a helper for heapam_scan_bitmap_next_tuple() --- 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 | 45 ++++++-------- src/backend/optimizer/util/plancat.c | 2 +- src/include/access/tableam.h | 75 +++++------------------ 6 files changed, 63 insertions(+), 111 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 3d92fb5135..8de1a11164 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -319,6 +319,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 867061325c..6af1791faa 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2109,12 +2109,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 * @@ -2148,8 +2142,8 @@ BitmapAdjustPrefetchIterator(HeapScanDesc 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->rs_base.prefetch_maximum > 0) { @@ -2512,30 +2506,43 @@ BitmapPrefetch(HeapScanDesc 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) { HeapScanDesc hscan = (HeapScanDesc) scan; OffsetNumber targoffset; Page page; ItemId lp; - if (hscan->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); - hscan->rs_empty_tuples_pending--; - return true; - } + if (hscan->rs_empty_tuples_pending > 0) + { + /* + * If we don't have to fetch the tuple, just return nulls. + */ + ExecStoreAllNullTuple(slot); + hscan->rs_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, &scan->blockno, + lossy_pages, exact_pages)) + return false; + } #ifdef USE_PREFETCH @@ -2917,7 +2924,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 ce637a5a5d..1d6b03d1ca 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 187b288e68..2f9387e51a 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -230,44 +230,35 @@ BitmapHeapNext(BitmapHeapScanState *node) #endif /* USE_PREFETCH */ node->initialized = true; - - goto new_page; } - 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, &scan->blockno, - &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 284ea3d864..44d0885d9e 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -788,36 +788,20 @@ typedef struct TableAmRoutine * ------------------------------------------------------------------------ */ - /* - * Prepare to fetch / check / return tuples from `blockno` 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 block's representation in the bitmap - * is lossy, 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, - BlockNumber *blockno, - 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 @@ -1998,44 +1982,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 if bitmap is lossy for the selected block and exact_pages is - * incremented otherwise. - * - * 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, BlockNumber *blockno, - 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, - blockno, 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 @@ -2046,7 +1999,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