Re: initscan for MVCC snapshot

Hi:
 I see initscan calls RelationGetwNumberOfBlocks every time and rescan calls
 initscan as well. In my system, RelationGetNumberOfBlocks is expensive (the reason
 doesn't deserve a talk.. ),  so in a nest loop + Bitmap heap scan case, the 

impact will be huge. The comments of initscan are below.

/*
* Determine the number of blocks we have to scan.
*
* It is sufficient to do this once at scan start, since any tuples added
* while the scan is in progress will be invisible to my snapshot anyway.
* (That is not true when using a non-MVCC snapshot.  However, we couldn't
* guarantee to return tuples added after scan start anyway, since they
* might go into pages we already scanned.  To guarantee consistent
* results for a non-MVCC snapshot, the caller must hold some higher-level
* lock that ensures the interesting tuple(s) won't change.)
*/

I still do not fully understand the comments. Looks we only need to call

multi times for non-MVCC snapshot, IIUC, does the following change reasonable?  

===

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 1b2f70499e..8238eabd8b 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -211,6 +211,7 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
        ParallelBlockTableScanDesc bpscan = NULL;
        bool            allow_strat;
        bool            allow_sync;
+       bool            is_mvcc = scan->rs_base.rs_snapshot && IsMVCCSnapshot(scan->rs_base.rs_snapshot);

        /*
         * Determine the number of blocks we have to scan.
@@ -229,7 +230,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
                scan->rs_nblocks = bpscan->phs_nblocks;
        }
        else
-               scan->rs_nblocks = RelationGetNumberOfBlocks(scan->rs_base.rs_rd);
+               if (scan->rs_nblocks == -1 || !is_mvcc)
+                       scan->rs_nblocks = RelationGetNumberOfBlocks(scan->rs_base.rs_rd);

        /*
         * If the table is large relative to NBuffers, use a bulk-read access
@@ -1210,6 +1212,7 @@ heap_beginscan(Relation relation, Snapshot snapshot,
        else
                scan->rs_base.rs_key = NULL;

+       scan->rs_nblocks = -1;
        initscan(scan, key, false);

--

Best Regards

Andy Fan