From 3382c9290f3c01389134156d95a9c92531748e48 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Tue, 18 Jun 2024 21:03:49 +0200 Subject: [PATCH v22b 05/29] review --- src/backend/executor/nodeBitmapHeapscan.c | 14 ++++++++++++++ src/include/access/tableam.h | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index e8b4a754434..6d7ef9ced19 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -270,6 +270,20 @@ new_page: BitmapAdjustPrefetchIterator(node); + /* + * XXX maybe add comment explaining what "false" means here? + * + * XXX I'm a bit unsure if this needs to be handled using goto. Wouldn't + * it be simpler / easier to understand to have two nested loops? + * + * while (true) + * if (!table_scan_bitmap_next_block(...)) { break; } + * while (table_scan_bitmap_next_tuple(...)) { + * ... process tuples ... + * } + * + * But I haven't tried implementing this. + */ if (!table_scan_bitmap_next_block(scan, &node->blockno, &node->recheck, &node->lossy_pages, &node->exact_pages)) break; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 2f4e221870e..c6a9fb34bd6 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -1023,6 +1023,7 @@ table_beginscan_analyze(Relation rel) static inline void table_endscan(TableScanDesc scan) { + /* XXX worth a comment? */ if (scan->rs_flags & SO_TYPE_BITMAPSCAN) { if (scan->shared_tbmiterator) @@ -1048,6 +1049,7 @@ static inline void table_rescan(TableScanDesc scan, struct ScanKeyData *key) { + /* XXX worth a comment? */ if (scan->rs_flags & SO_TYPE_BITMAPSCAN) { if (scan->shared_tbmiterator) -- 2.45.2