agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Melanie Plageman <[email protected]>
Subject: [PATCH v8 15/17] Remove table_scan_bitmap_next_block()
Date: Fri, 22 Mar 2024 15:43:10 -0400
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_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 +++++------------------
5 files changed, 61 insertions(+), 111 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 1a9f7b02d1..9dfc99d8fa 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2110,12 +2110,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)
{
@@ -2511,30 +2505,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
@@ -2916,7 +2923,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 b548642088..2683d8bc0c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -222,44 +222,35 @@ BitmapHeapNext(BitmapHeapScanState *node)
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 2ded1a124b..5ad3eff539 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
@@ -2000,44 +1984,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
@@ -2048,7 +2001,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
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0016-v7-Streaming-Read-API.patch"
view thread (12+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v8 15/17] Remove table_scan_bitmap_next_block()
In-Reply-To: <no-message-id-1859437@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox