fix tids are not in order when building bitmap index (#15271)

When we build a bitmap index on a heap table, we need to scan all the
tuples. Usually, it'll start at block 0 to the end. But if we allow sync scan,
it'll scan the table circularly, from block X up to the end and then from
block 0 to X-1, to ensure we visit all rows while still participating in the
common scan. This could lead tids are not in order when building bitmap
index, which will throw an error in the current implementation.

So this PR forbids sync scan when building bitmap index.

Besides the above fact, if we build bitmap index on a heap table, there could
have HOT-chain, and it'll return the root tuple's tid, which could lead to the
TIDs we scanned are not in order.

Author: SmartKeyerror

src/backend/access/bitmap/bitmap.c

@@ -145,7 +145,7 @@ bmbuild(Relation heap, Relation index, IndexInfo *indexInfo)
 	_bitmap_init_buildstate(index, &bmstate);
 
 	/* do the heap scan */
-	reltuples = table_index_build_scan(heap, index, indexInfo, true, true,
+	reltuples = table_index_build_scan(heap, index, indexInfo, false, true,
 									   bmbuildCallback, (void *) &bmstate,
 									   NULL);
 	/* clean up the build state */

src/backend/access/bitmap/bitmapinsert.c

@@ -1790,8 +1790,24 @@ buf_add_tid(Relation rel, BMTidBuildBuf *tids, uint64 tidnum,
 		buf = lov_buf->bufs[off - 1];
 
 		Buffer lovbuf = _bitmap_getbuf(rel, lov_block, BM_WRITE);
-		buf_add_tid_with_fill(rel, buf, lovbuf, off,
-							  tidnum, state->use_wal);
+
+		if (tidnum < buf->last_tid)
+		{
+			/*
+			 * Usually, tidnum is greater than lovItem->bm_last_setbit.
+			 * However, if we build bitmap index on a heap table, there could
+			 * have HOT-chain, and it'll return the root tuple's tid, which could
+			 * lead to the TIDs we scanned are not in order, so we need to scan
+			 * through the bitmap vector, and update the bit in tidnum directly.
+			 */
+			_bitmap_write_new_bitmapwords(rel, lovbuf, off, buf, state->use_wal);
+			_bitmap_free_tidbuf(buf);
+
+			updatesetbit(rel, lovbuf, off, tidnum, state->use_wal);
+		}
+		else
+			buf_add_tid_with_fill(rel, buf, lovbuf, off, tidnum, state->use_wal);
+
 		_bitmap_relbuf(lovbuf);
 	}
 	else
@@ -1819,8 +1835,7 @@ buf_add_tid(Relation rel, BMTidBuildBuf *tids, uint64 tidnum,
 
 		buf->curword = 0;
 
-		buf_add_tid_with_fill(rel, buf, lovbuf, off, tidnum,
-							  state->use_wal);
+		buf_add_tid_with_fill(rel, buf, lovbuf, off, tidnum, state->use_wal);
 
 		_bitmap_relbuf(lovbuf);
 
@@ -1851,19 +1866,6 @@ buf_add_tid_with_fill(Relation rel, BMTIDBuffer *buf,
 	 */
 	zeros = tidnum - buf->last_tid - 1;
 
-	/*
-	 * If zeros is less than 0, the incoming tids are not
-	 * sorted. Currently, this is not allowed.
-	 */
-	if (zeros < 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("tids are not in order when building bitmap index %u (relfilenode %u/%u/%lu):"
-						" new tidnum " INT64_FORMAT ", last tidnum " INT64_FORMAT,
-						RelationGetRelid(rel),
-						rel->rd_node.spcNode, rel->rd_node.dbNode, rel->rd_node.relNode,
-						tidnum, buf->last_tid)));
-
 	if (zeros > 0)
 	{
 		uint64 zerosNeeded;

src/test/regress/expected/bitmap_index.out

@@ -1061,3 +1061,30 @@ select gp_inject_fault('simulate_bitmap_and', 'reset', dbid) from gp_segment_con
 reset optimizer_enable_tablescan;
 reset optimizer_enable_dynamictablescan;
 drop table bmunion;
+-- test create bitmap index and there have HOT chains.
+drop table if exists bm_test;
+NOTICE:  table "bm_test" does not exist, skipping
+create table bm_test(a int, b int);
+NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
+HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
+-- insert some data into a one segment
+insert into bm_test values (1, 1); 
+insert into bm_test values (1, 2); 
+insert into bm_test values (1, 3); 
+insert into bm_test values (12, 1); 
+-- update the first tuple using HOT, since this page
+-- just have 4 tuples, there have full free space to
+-- use HOT update.
+update bm_test set b = 1 where a = 1 and b = 1;
+-- After the update, the tids that the value of b is equal to 1
+-- we scanned will not be in order, due to HOT.
+create index idx_bm_test on bm_test using bitmap(b);
+select * from bm_test where b = 1;
+ a  | b 
+----+---
+  1 | 1
+ 12 | 1
+(2 rows)
+
+-- clean up
+drop table bm_test;

src/test/regress/expected/bitmap_index_optimizer.out

@@ -1072,3 +1072,30 @@ select gp_inject_fault('simulate_bitmap_and', 'reset', dbid) from gp_segment_con
 reset optimizer_enable_tablescan;
 reset optimizer_enable_dynamictablescan;
 drop table bmunion;
+-- test create bitmap index and there have HOT chains.
+drop table if exists bm_test;
+NOTICE:  table "bm_test" does not exist, skipping
+create table bm_test(a int, b int);
+NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
+HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
+-- insert some data into a one segment
+insert into bm_test values (1, 1); 
+insert into bm_test values (1, 2); 
+insert into bm_test values (1, 3); 
+insert into bm_test values (12, 1); 
+-- update the first tuple using HOT, since this page
+-- just have 4 tuples, there have full free space to
+-- use HOT update.
+update bm_test set b = 1 where a = 1 and b = 1;
+-- After the update, the tids that the value of b is equal to 1
+-- we scanned will not be in order, due to HOT.
+create index idx_bm_test on bm_test using bitmap(b);
+select * from bm_test where b = 1;
+ a  | b 
+----+---
+  1 | 1
+ 12 | 1
+(2 rows)
+
+-- clean up
+drop table bm_test;

src/test/regress/sql/bitmap_index.sql

@@ -449,3 +449,27 @@ reset optimizer_enable_tablescan;
 reset optimizer_enable_dynamictablescan;
 
 drop table bmunion;
+
+
+-- test create bitmap index and there have HOT chains.
+drop table if exists bm_test;
+create table bm_test(a int, b int);
+
+-- insert some data into a one segment
+insert into bm_test values (1, 1); 
+insert into bm_test values (1, 2); 
+insert into bm_test values (1, 3); 
+insert into bm_test values (12, 1); 
+
+-- update the first tuple using HOT, since this page
+-- just have 4 tuples, there have full free space to
+-- use HOT update.
+update bm_test set b = 1 where a = 1 and b = 1;
+
+-- After the update, the tids that the value of b is equal to 1
+-- we scanned will not be in order, due to HOT.
+create index idx_bm_test on bm_test using bitmap(b);
+select * from bm_test where b = 1;
+
+-- clean up
+drop table bm_test;