Fix bitmapheapscan incorrect recheck of NULL tuples

The bitmap heap scan skip fetch optimization skips fetching the heap
block when a page is set all-visible in the visibility map and no
columns from the table are needed to satisfy the query.

2b73a8cd33b and c3953226a07 changed the control flow of bitmap heap scan
to use the read stream API. The read stream API returns buffers
containing blocks to the user. To make this work with the skip fetch
optimization, we keep a count of the empty tuples we need to emit for
all the blocks skipped and only emit the empty tuples after processing
the next block fetched from the heap or at the end of the scan.

It's incorrect to recheck NULL tuples, so we must set `recheck` to false
before yielding control back to BitmapHeapNext(). This was done before
emitting any remaining empty tuples at the end of the scan but not for
empty tuples emitted during the scan. This meant that if a page fetched
from the heap did require recheck and set `recheck` to true and then we
emitted empty tuples for subsequent blocks, we would get wrong results.

Fix this by always setting `recheck` to false before emitting empty
tuples.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Tested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/496f7acd-881c-4df3-9bd3-8f8534dfec26%40gmail.com

2 files changed, 20 insertions, 4 deletions

diff --git a/src/test/regress/expected/bitmapops.out b/src/test/regress/expected/bitmapops.out
index 3570973e3ca..64068e0469c 100644
--- a/src/test/regress/expected/bitmapops.out
+++ b/src/test/regress/expected/bitmapops.out
@@ -8,7 +8,7 @@
 -- there's a maximum number of a,b combinations in the table.
 -- That allows us to test all the different combinations of
 -- lossy and non-lossy pages with the minimum amount of data
-CREATE TABLE bmscantest (a int, b int, t text);
+CREATE TABLE bmscantest (a int, b int, t text) WITH (autovacuum_enabled = false);
 INSERT INTO bmscantest
   SELECT (r%53), (r%59), 'foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo'
   FROM generate_series(1,70000) r;
@@ -20,7 +20,17 @@ set enable_indexscan=false;
 set enable_seqscan=false;
 -- Lower work_mem to trigger use of lossy bitmaps
 set work_mem = 64;
--- Test bitmap-and.
+-- Test bitmap-and without the skip fetch optimization.
+SELECT count(*) FROM bmscantest WHERE a = 1 AND b = 1;
+ count 
+-------
+    23
+(1 row)
+
+-- Test that we return correct results when using the skip fetch optimization
+-- VACUUM FREEZE will set all the pages in the relation all-visible, enabling
+-- the optimization.
+VACUUM (FREEZE) bmscantest;
 SELECT count(*) FROM bmscantest WHERE a = 1 AND b = 1;
  count 
 -------
diff --git a/src/test/regress/sql/bitmapops.sql b/src/test/regress/sql/bitmapops.sql
index 498f4721b51..1b175f6ff96 100644
--- a/src/test/regress/sql/bitmapops.sql
+++ b/src/test/regress/sql/bitmapops.sql
@@ -12,7 +12,7 @@
 -- That allows us to test all the different combinations of
 -- lossy and non-lossy pages with the minimum amount of data
 
-CREATE TABLE bmscantest (a int, b int, t text);
+CREATE TABLE bmscantest (a int, b int, t text) WITH (autovacuum_enabled = false);
 
 INSERT INTO bmscantest
   SELECT (r%53), (r%59), 'foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo'
@@ -29,8 +29,14 @@ set enable_seqscan=false;
 -- Lower work_mem to trigger use of lossy bitmaps
 set work_mem = 64;
 
+-- Test bitmap-and without the skip fetch optimization.
+SELECT count(*) FROM bmscantest WHERE a = 1 AND b = 1;
+
+-- Test that we return correct results when using the skip fetch optimization
+-- VACUUM FREEZE will set all the pages in the relation all-visible, enabling
+-- the optimization.
+VACUUM (FREEZE) bmscantest;
 
--- Test bitmap-and.
 SELECT count(*) FROM bmscantest WHERE a = 1 AND b = 1;
 
 -- Test bitmap-or.